diff --git a/builtin/runtime/linewrap.lua b/builtin/runtime/linewrap.lua new file mode 100644 index 0000000..39aae65 --- /dev/null +++ b/builtin/runtime/linewrap.lua @@ -0,0 +1,73 @@ +-- Long lines (QoL Stage 3, framing docs/long-lines-framing.md). +-- +-- Declares `ui.line-wrap`. Everything that honors it is Rust: the grid +-- renderer walks it through `Viewport`, the coordinate mapping takes it +-- through `LayoutCtx`, and semantic frontends are told over +-- `InstanceMessage::LineWrapFacts` at protocol v22 because they lay out +-- locally and would otherwise never hear it. +-- +-- Buffer-local (Q#LL2). The registry already supports a per-buffer +-- layer, and this is a property of the content: prose wants wrapping, +-- a log file usually does not. The *anchor* the viewport scrolls to +-- stays per-window, because two panes on one buffer scroll +-- independently. +-- +-- `ui.`, not `editing.`: `editing.*` is buffer-editing behavior +-- (auto-pair, trim-on-save, line endings) and this changes only how +-- text is shown. The two existing `ui.*` settings carry a `gpu-` +-- prefix to mark frontend-specific ones, so the ABSENCE of a prefix +-- here is what says "both frontends". + +pmacs.config.define { + name = "ui.line-wrap", + description = "How a line wider than the window is shown: wrap onto following rows, or truncate at the edge.", + -- A closed set, so an unknown value is impossible rather than + -- handled. Adding "word" later is a clean additive change --- which + -- is the plan, since character wrap is what both frontends can do + -- identically today (Q#LL5) and word wrap is a deliberate future + -- choice rather than an inherited library default. + type = "enum", + choices = { "wrap", "truncate" }, + -- `wrap` is the only value that leaves every character reachable + -- with this stage's machinery. It is also what the GPU already did, + -- so the default is not a behavior change there --- but it IS one in + -- the TUI, which truncated. No default can preserve both, because + -- the two frontends disagreed before this setting existed; that is + -- the defect, not a side effect of fixing it. + default = "wrap", + mutability = "live", +} + +-- `truncate` leaves text past the right edge UNREACHABLE until Stage 4 +-- adds horizontal scrolling. That is stated in the description above +-- rather than left for a user to discover, and it is why `truncate` is +-- not the default despite being the TUI's historical behavior. + +-- Buffer-local on BOTH sides, and the pairing matters. +-- +-- `pmacs.config.get(name)` reads the global chain and +-- `pmacs.config.set(name, ...)` writes the global layer, so a toggle +-- built from those two would be wrong in the case the setting exists +-- for: a buffer pinned to `truncate` would report the GLOBAL value, +-- flip the GLOBAL value, and leave that buffer exactly as it was --- +-- while silently changing every buffer that had not been pinned. +-- +-- So: read with `get(name, buf)`, write with `set_local(buf, ...)`, +-- and resolve the buffer ONCE for both. Resolving twice would be a +-- narrower version of the same bug, since the active buffer can change +-- between two calls. +pmacs.command.define { + name = "ui.toggle-line-wrap", + description = "Toggle line wrapping for the current buffer", + fn = function() + local buf = pmacs.window.buffer() + local current = pmacs.config.get("ui.line-wrap", buf) + local next_mode = current == "wrap" and "truncate" or "wrap" + pmacs.config.set_local(buf, "ui.line-wrap", next_mode) + if next_mode == "truncate" then + pmacs.editor.set_status("line wrap off — text past the edge is unreachable until horizontal scrolling lands") + else + pmacs.editor.set_status("line wrap on") + end + end, +} diff --git a/docs/active-work.md b/docs/active-work.md index 9fc9b2c..b00ed4d 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -196,7 +196,7 @@ hazard in a shape that looks committed. **A documented error message that never appears is worse than no documentation**, because the reader waits for a signal that is not coming. -## Docs absorption after #217 — PR #218 OPEN +## Docs absorption after #217 — MERGED as #218 (2026-08-06 09:59Z) **PR #218** — https://github.com/levineuwirth/pmacs/pull/218. **This block was written with the lane's first commit, before the PR @@ -225,7 +225,7 @@ are the lanes this one creates, not work it does. Retiring the CI-CRDT, Distribution, or reap-ledger lanes: each still owns undone work and rule 4 does not apply to them. -## Honoring `full_grid` (QoL Stage 1) — PR #219 OPEN +## Honoring `full_grid` (QoL Stage 1) — MERGED as #219 (2026-08-06 13:41Z) **PR #219** — https://github.com/levineuwirth/pmacs/pull/219. **This block was written with the lane's first commit, before the PR @@ -296,7 +296,7 @@ change to *when* `needs_full_grid` is set — the producer's triggers were verified correct, along with per-frame geometry sync and `view_top` reconciliation on shrink. -## GUI zoom (QoL Stage 2) — PR #220 OPEN +## GUI zoom (QoL Stage 2) — MERGED as #220 (2026-08-07 08:25Z) **PR #220** — https://github.com/levineuwirth/pmacs/pull/220. **Written with the lane's first commit, before the PR existed** — the standing @@ -434,6 +434,278 @@ 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) — PR #221 OPEN, awaiting review + +**Branch `long-lines`**, originally based on `githubsucks/main` @ +`218d2e7` (the #219 merge); **main merged in at #220's landing**, so +the branch now carries Stage 2. The two share no code — that merge is +currency, not dependency, taken so the Stage 3 PR opens against a base +it has already been tested on. `githubsucks/long-lines` is the authoritative tip — the ref, +not a SHA, since any edit to this block advances past whatever SHA it +records. Recover: `git fetch githubsucks && git checkout long-lines`. + +**This block was written with the lane's first commit, before any PR +existed** — the standing correction from #171, #215 and #220. **PR +#221** opened 2026-08-07 against `main` @ `912bf57`; the number is +recorded here rather than left to be reconstructed from the branch. + +- **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 + +A line wider than the window is unreadable past the edge **in the +TUI**. The GPU is not in that state: it already wraps. So the +cross-frontend defect is **not** unreadability — it is that *neither +behavior was chosen*. The TUI truncates because a cell walk breaks at +`max_cols`; the GPU wraps because cosmic-text's `Wrap::WordOrGlyph` +default was never overridden. Two accidents that disagree, with no way +for the user to express a preference in either. + +### The six answers + +- **Q#LL1** — ships `wrap` + `truncate`, **default `wrap`**; horizontal + scroll is **Stage 4**. No default preserves both frontends, so this + knowingly changes the TUI's behavior and leaves the GPU's alone. +- **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`; 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 + layout is per-line); `view_top`'s sub-line component is a **byte**; + `DisplayCoord` gains `sub_row` rather than redefining `row`. + +### The two holes review found in the first commit + +- **Q#LL7 --- the GPU had no wire.** The mode resolved into `Viewport`, + which reaches only the *grid* renderer; the GPU lays out locally and + no message expresses a wrap mode. `truncate` would have changed the + TUI and left the GPU wrapping --- the exact disagreement this lane + exists to close. Now specified as an additive v22 variant carrying + `buffer_id`, resent on attach, config change, **and buffer switch**. + That third trigger is the subtle one: font size is global, wrap mode + is per buffer, so a `FontFacts`-shaped design is silently wrong and + looks right in every single-buffer test. +- **Q#LL8 --- "every vertical consumer is local" was false.** The + scroll indicator needs a total. A one-line buffer wrapping to fifty + rows has `total_lines == 1`, so the indicator reports `All` while + forty-nine rows sit off-screen. The bounded distinction survives: a + *total* is one lazily-computed number; an *index* is `O(N)` resident + storage, still ruled out. + + **Two further corrections, from review of the second commit.** + `format_scroll_indicator` is **duplicated, not shared** + (`src/editor.rs:5509`, `pmacs-gpu/src/main.rs:10114`), and the GPU + passes a source-line count --- so the first fix would have corrected + one frontend and left the other wrong, *this lane's own defect + reproduced by the section meant to close it*. And the lazy total's + cache key omitted **fold state**, which changes per window with no + edit, resize or mode change; now keyed on the fold projection's own + contents, which cannot be forgotten, rather than a maintained + revision counter that can. + + **Then the aggregate was abandoned entirely (revision 17).** The GPU + shapes **only the viewport slice** — Session S1 found the whole rope + made large-file editing `O(file)` per keystroke — so its layout + cannot yield a total, and re-shaping the document for a status-line + readout would reintroduce that cost. `NN%` is now **byte-based in + both frontends**; `All`/`Top`/`Bot` stay exact because they are local + predicates. This retires the cache *and* the fold-key fix above, + which is kept in the framing marked superseded so a reader can tell + "the key was fixed" from "there is no key". + + **Then the retained formatter turned out unable to express the new + contract (revision 18).** Every branch of `format_scroll_indicator` + derives from `total_lines`, so a byte total would compare rows + against bytes and a fake total restores the false `All`. Resolved by + keeping the existing formatter **untouched for `truncate`** — output + identical by construction — and adding a `classify(first_visible, + last_visible, byte_pos, byte_len)` for `wrap` that never sees a row + count. **Resolved: the classifier lives in `pmacs-protocol`** — + `ScrollPosition` plus a pure `classify`, with string rendering left + in each frontend. Per §16: each frontend computes its own local + layout facts, the shared crate owns the semantic decision they feed. + **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 + Emacs-consistent, but the GPU has word-wrapped since it existed. + Accepted deliberately (user, 2026-08-06). **Must appear in the PR + description and release notes**, not only in the framing. +- **The audit strategy is asymmetric on purpose.** The coordinate + functions gain a required context parameter so the **compiler + enumerates** every call site; `DisplayCoord` gains an **additive** + field so untouched consumers stay *correct* rather than merely + findable. Compiler-enforced where possible, correct-by-default where + not. + +### Verification (delivered) + +Everything framing §7 sketched, plus three groups it did not anticipate +(framing §7.1). + +**The gate list below is the one that FAILED to catch this lane's CI +red, and it is kept only to show the hole.** "The touched acceptance +suites" is selected from the diff, and a `PROTOCOL_VERSION` bump breaks +version-assertion tests that appear nowhere in it. Five failed on CI's +first round. Use **`cargo test --tests --no-fail-fast`** on any +protocol bump — `--no-fail-fast` because cargo stops at the first +failing target, which is why CI showed one failure and the local +full-corpus run then found three more. Recorded in +`docs/agent-handoff.md`. + +``` +cargo fmt --check +cargo clippy --workspace --all-targets -- -D warnings +cargo clippy --workspace --all-targets --features crdt -- -D warnings +cargo test --lib # 1917 / 0 +cargo test --lib --features crdt # 2102 / 0 +cargo test --tests --no-fail-fast # 108 targets, exit 0 +cargo test --tests --features crdt --no-fail-fast # 108 targets, exit 0 +PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu # 228 / 0 +git diff --check +``` + +**Both configurations, and that mattered.** The default sweep went +clean while `--features crdt` still had **three** failures, in +crdt-gated real-daemon tests (`vterm_stage3_acceptance` ×2, +`bottom_panel_stage2b_gpu_acceptance` ×1) that assert on a real +socket's negotiated version. This is the handoff's existing "a local +sweep is blind to whichever feature configuration it does not build" +lesson, hit again by a different lane — so **eight** version +assertions broke in total, not five. + +The eight version-assertion failures, and what each one was: + +| test | was | why it broke | +|---|---|---| +| `acc51_a_v20_peer_..._even_when_capable` (×5 jobs) | `PROTOCOL_VERSION - 1` | **bug** — an absolute contract ("below the panel version") as arithmetic on a moving constant. Now `PANEL_MIN_VERSION - 1`, the idiom `src/` already used in five places | +| `the_baseline_stays_and_the_counter_offer_activates` | `PANEL_MIN_VERSION == PROTOCOL_VERSION` | **bug** — asserted a coincidence true only while panels were the newest feature. Now the two durable bounds | +| `the_panel_stage_takes_protocol_v21` | `PROTOCOL_VERSION == 21` | **bug** — the current wire as a proxy for the panel stage's own version, in a test whose name says which it means | +| `a54_real_daemon_..._panel_hosted_terminal` *(crdt)* | `session_protocol_version == "21"` | **bug** — the negotiated session version is *this binary's* wire, so the literal held only while panels were newest. Now `PROTOCOL_VERSION`, plus an explicit `>= PANEL_MIN_VERSION` for the capability the literal carried implicitly | +| `a37_real_daemon_..._one_terminal_session` *(crdt)* | `session_protocol_version == "21"` | **bug** — same | +| `a13_17_26_..._version_cost` | `PROTOCOL_VERSION == 21`, `6..=21`, `!supported(22)` | **tripwire working as designed** — it says in its own comment that it tracks the current wire. Took the conscious edit | +| `the_baseline_stays_...` (same test) | `PROTOCOL_VERSION == 21` | **tripwire working as designed** | +| `terminal_mode_keeps_reporting_presence_...` *(crdt)* | `PROTOCOL_VERSION == 21` | **tripwire working as designed** | + +**Five bugs and three tripwires.** The distinction is the useful part. +A tripwire that fires on a bump is doing its job, and "fixing" it means +editing it deliberately — the baseline pin `ADVERTISED_PROTOCOL_VERSION +== 20` is the one that must *never* be edited, and it never fired. + +The five bugs share one shape: **an absolute contract expressed as +arithmetic on, or equality with, a moving constant.** `PROTOCOL_VERSION +- 1` for "below the panel version"; `PANEL_MIN_VERSION == +PROTOCOL_VERSION` for a coincidence; `PROTOCOL_VERSION == 21` standing +in for the panel stage's own version; `"21"` for a negotiated session +version. Each was true when written and silently false afterwards. +`src/` already had the right idiom — `PANEL_MIN_VERSION - 1`, five +occurrences — and every outlier was in `tests/`. + +`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. + +**And one more, logged as U2.** +`process::tests::m6_1_pty_raw_mode_disables_kernel_echo` failed once +during a full `--tests --no-fail-fast` run and did not reproduce in a +later full sweep (108 targets, exit 0) or 3 isolated `--lib` runs. It +is in no registry row, so it is a new incident; leaked +`pmacs --daemon` processes remain an unexcluded rival explanation. + +### Not in scope + +Horizontal scroll in full (Stage 4). `M-q` / auto-fill / reflow. Word +wrap as a mode value — a named future third choice, not this stage. +Bidi/RTL. Soft-wrap gutter indicators. ## Tree primitive (P5) — MERGED as #217; adoption is the open work diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index e94db13..48a29fd 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -404,6 +404,35 @@ someone forgot. old. They are a rival explanation for any load-sensitive local failure, so **check `pgrep -f "pmacs --daemon"` before trusting a local red**. Lane recorded in `docs/active-work.md`. +- **A `PROTOCOL_VERSION` bump's blast radius is every version-sensitive + test, and NONE of them appear in the diff.** "The touched acceptance + suites" is the standing gate, and for a protocol bump it is the wrong + selector: long-lines Stage 3 bumped v21→v22, ran the suites it had + edited, and broke **eight** version assertions across six suites. CI + showed exactly **one**, because **cargo stops at the first failing + target**; the rest surfaced only afterwards, and one at a time would + have cost four more red rounds. + + **On any protocol bump run `cargo test --tests --no-fail-fast` in + BOTH feature configurations.** `--no-fail-fast` because of the + stop-at-first-target behavior above, and `--features crdt` because + three of the eight were in crdt-gated real-daemon tests that assert + on a live socket's negotiated version — invisible to a default sweep, + which is the blindness the bullet below already names. + + **Sort the failures before fixing them.** A *tripwire* + (`assert_eq!(PROTOCOL_VERSION, N)`) is meant to fire and takes a + deliberate edit; three of the eight were these, and the one pin that + must NEVER be edited — `ADVERTISED_PROTOCOL_VERSION == 20` — did not + fire. The other five were defects sharing one shape: **an absolute + contract expressed as arithmetic on, or equality with, a moving + constant.** `PROTOCOL_VERSION - 1` for "below the panel version"; + `PANEL_MIN_VERSION == PROTOCOL_VERSION` for a coincidence true only + while panels were newest; `PROTOCOL_VERSION == 21` standing in for + the panel stage's own version; `"21"` for a negotiated session + version. Anchor on the constant the contract *names* — `src/` already + spelled it `PANEL_MIN_VERSION - 1` in five places, and every outlier + was in `tests/`. - **A local sweep is blind to whichever feature configuration it does not build.** Stage 3's census and every verification sweep ran `--features luajit` WITHOUT `crdt`, so no crdt-gated suite was diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 66e9949..0f2154b 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -393,6 +393,51 @@ 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. + +### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, one local occurrence + +Has a selector, which U1 lacks — but still no fragments, so it cannot +be matched either. Recorded so a recurrence is recognisable. + +| field | value | +|---|---| +| **selector** | `--lib process::tests::m6_1_pty_raw_mode_disables_kernel_echo` | +| **job / flavor** | local (Linux), during `cargo test --tests --no-fail-fast` — the lib target alongside a full PTY-heavy corpus | +| **required fragments** | **none captured** — output was filtered to the `FAILED` line | +| **status** | **new incident, unreproduced** | +| **what IS established** | it failed once (`1916 passed; 1 failed`), in no registry row, under a full-corpus run | +| **what is NOT** | any mechanism. Not reproduced in a later full `--tests --no-fail-fast` sweep (108 targets, exit 0) nor in 3 isolated `--lib` runs (1917/0 each) | +| **rival explanation not excluded** | leaked `pmacs --daemon` processes, which the handoff names as a standing confound for any load-sensitive local red | + **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 new file mode 100644 index 0000000..fe6a13c --- /dev/null +++ b/docs/long-lines-framing.md @@ -0,0 +1,1838 @@ +# Long lines — QoL Stage 3 + +**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 +ignores the grid variants and lays out locally, and it **already wraps +long lines**. See §1.2. The correction reverses the cost profile and +therefore the recommendation in §3. + +**Revision 3** fixed two things review caught downstream of that +correction. The opening still called the defect "total ... in either +frontend," contradicting §1.2's own table — corrected below, along with +what the cross-frontend defect actually is. And §7's GPU witness +described the non-wrap mode as "scroll/truncate," conflating a deferred +Stage 4 capability with a Stage 3 mode; **§3.1 now states the Stage 3 +surface explicitly** rather than letting a test mode imply a product +one. + +Writing §3.1 surfaced a third thing neither review nor revision 2 had +named: agreeing on the mode is not agreeing on the wrap. **New Q#LL5 +(§5a)** — the GPU wraps at `Wrap::WordOrGlyph` and a grid walk would +naturally wrap at the character, so both frontends could honor `wrap` +and still break lines in different places. It was briefly buried in +"not in scope"; it is the same class of defect this lane exists to +close, so it is a question now. + +**Revision 4** — the verification sketch still asked +for `pos_to_display` / `display_to_pos` round trips "at non-zero +offset", and its first bullet for cell tests "at several offsets". +Both are `view_left` requirements, and `view_left` is Stage 4. +Deferring scope in §3.1 and §9 while test lines quietly assumed it is +exactly how deferred work creeps back in. Replaced with the wrapped +visual-row mapping witness `wrap` actually needs — including the +wrap-point boundary case and a `truncate` control — and with "at +several window widths" (§7). + +**Revision 5** — revision 4's replacement then asked +for round-trip identity "for every position", which the existing +coordinate contract makes impossible: `pos_to_display` canonicalizes a +byte inside a multi-byte codepoint to that codepoint's column, and +`display_to_pos` returns the codepoint start. Restated as identity on +valid cursor boundaries plus projection elsewhere, with the +interior-byte canonicalization preserved under its own separate witness +(§7). A witness that cannot be satisfied gets weakened until it passes, +which would have cost exactly the discriminating power §7 exists for. + +**Revision 6** — the wrap-point case still said the +last position on row *k* and the first on row *k+1* "must not +collide". They are **one** source position with two candidate +coordinates, so that assertion has no content. §7 now *decides* the +ownership — the wrap position is column 0 of the next row, because the +alternative coordinate is off-grid on a row that is full by +construction — states affinity as a deliberate non-goal, and separately +names the requirement revision 5 was actually reaching for: the two +*distinct* adjacent codepoint starts across the break must map +distinctly. + +**Revision 7** — revision 6's justification was wrong +twice. A hard line ending at exactly `max_cols` also has column +`max_cols` — `pos_to_display` clamps nothing — so "off-grid" never +distinguished the soft-wrap case; and `pos_to_display` takes no +viewport at all, so it has no grid to be off. The decision stands, on a +rule that subsumes both cases: **a position maps to the cell of the +glyph that follows it when one exists, otherwise just past the last +glyph** — which resolves the soft wrap and leaves hard ends, including +full-row ones, exactly as they are today (now a control in §7). + +Chasing that also surfaced a structural cost no earlier revision had: +under `wrap`, `pos_to_display` **cannot compute a visual row from its +current arguments**, so the wrap width has to reach it — a trait +signature change across ~35 call sites, though only `TextView` +overrides the method. + +**Revision 8** — that cost was still framed too +narrowly: a signature is not a model. `display_to_pos` has the same +missing inputs and treats `coord.row` as a raw source-line index, so it +fails *silently* into the wrong line; and `view_top`, vertical motion, +paging, wheel scroll, gutters and overlays all operate in source-line +space today. `move_down` alone treats a display row as a source line. +**New Q#LL6 (§5b)** makes the +authoritative logical-to-visual row map a design item — both +directions, its width/mode inputs, how it *composes with* the existing +fold map rather than bypassing it, what becomes of `view_top` (a +persisted value, via `saveplace`), and `truncate` as the identity case. +§5b.7 restates the cost as an audit of both mapping APIs and every +source-row assumption. + +**Revision 9** — revision 8's `view_top` question was +a **false binary**: "source line" and "visual row" are both +unworkable. A source line cannot name a viewport starting partway down +a wrapped line, so a line taller than the viewport could never scroll +to its second visual row — the exact buffers this lane is for. The +representation must be composite (anchor line + row-within-line), +composed with folds. + +The persistence consequence is also sharper than "a format change". +`saveplace` has **no version marker** and stores a bare integer, so +redefining `view_top` silently reinterprets every existing record; and +its path field is the whitespace-split remainder, so appending a field +is not backward-compatible either. A visual row is additionally +**width-dependent** — saved at 120 columns, restored at 80, it denotes +a different place. §5b.6 is new, recommends persisting only the +width-independent anchor line (no migration needed, by construction), +and requires Q#LL6 to settle a **resize-restore policy** for the +row-within-line offset, which live resizes need regardless of what is +stored. + +Also: revisions 4 through 8 each ended up labelled "the current one". +Only the Status line above is authoritative; the stale markers are +removed. + +**Revision 10** — revision 9 described +`VisibleLineMap` as mapping source lines to a renumbered *visible-line* +space. It does not: `next_visible`, `prev_visible`, `visible_head_of` +and `clamp_view_top` all take and return **source-line indices**, +constrained to visible heads (`src/fold_view.rs:223`). Folds project +onto visible anchors; they do not renumber. The error mattered in the one place +this section exists to protect — a reader who believed folds renumber +would add a *second* renumbering for wrap and misindex every fold +consumer. §5b.2, §5b.3 and §5b.4 are corrected, and the same loose +wording is fixed in §2.1, §5 and §7, where it had also mislabelled +`pos_to_display`'s current return (a **source** line index, from +`line_at_offset`). + +§5b.6 additionally turns the anchor-persistence recommendation into an +explicit **public API contract**: `pmacs.editor.view_top()` keeps +returning the source anchor and `set_view_top(n)` sets it with +`row_within_line = 0`, so `saveplace` needs no change and existing +records keep working by contract rather than by luck. + +**Revision 11** — revision 10 replaced "renumbers" +with "restricts the source-line domain to visible heads". The second +half is also wrong, and in a way that inverts a contract: +`clamp_view_top` **deliberately accepts a hidden line** and projects it +to its visible head — that is why it exists (`src/fold_view.rs:215`), +and `text_view::render` depends on it. "Restricted domain" would make +the supported case read as a caller error. §5b.3 now describes the +first step as a **source-index-preserving projection onto a visible +source-line anchor**: total, idempotent, same index space — which is +the same shape as §7's coordinate rule one level up. The load-bearing +conclusion is unchanged: no dense middle coordinate space exists. + +**Revision 12** — revision 11 stated that shared rule +as "projection to the **nearest** canonical value". It is not +proximity-based: a hidden line maps to its fold head even when the next +visible line is closer, and an interior UTF-8 byte maps to its +codepoint start rather than the nearer boundary. Nor is it uniformly +backward — `pos_to_display` projects an interior byte back to the +codepoint start while `display_to_pos` rounds forward to the next +boundary. The accurate rule is **identity on canonical inputs; +otherwise projection to the contract's designated canonical +representative**, with total-and-idempotent as the genuinely shared +algebra (§5b.3). + +**Revision 13** — records decisions from the +2026-08-06 design discussion rather than correcting an error. + +**Q#LL1 is answered** (§3): `wrap` + `truncate`, **default `wrap`**, +scroll deferred to Stage 4. The default is a knowing behavior change to +the TUI — no default can preserve both frontends, because they +currently disagree. + +**Q#LL6 item 3 is answered** (§5b.4): `view_top`'s sub-line component +is a **byte**, not a row index — width-independent, exactly reversible +across resizes, and it **dissolves** the resize-restore policy §5b.6 +demanded rather than answering it. Two further structural decisions are +recorded in the new §5b.5: **no global map is needed** (every vertical +consumer is local, so layout is per-line — which removes the +`open_100mb_under_200ms` risk §5b.7's framing invited), and +**`DisplayCoord` gains a `sub_row` rather than redefining `row`**, so +untouched consumers stay correct instead of merely findable. + +Found while answering: **the GPU already carries this composite +anchor** — `scroll_top` plus `code_scroll_residual`, renormalized by +`normalize_code_scroll` (framing Q#F6) when reflow pushes the residual +across source lines. The shape is precedent, not invention. + +**Revision 14** — answers the remaining four questions +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`; +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 +is worse than honest divergence. **Q#LL6 items 1-2** (§5b.4): +`TextView` methods, no cache initially, one `Copy` context parameter — +breaking on the input side so the compiler enumerates the audit, +additive on the output side so untouched consumers stay correct. + +**Revision 15** — review of `bd752f2` found two holes +and one notation hazard. + +**Q#LL7 (§5c) — the GPU had no wire.** §4 resolves the mode into +`Viewport`, which reaches the *grid*. The GPU is not a grid consumer: +it lays out locally, `BufferSnapshot` carries only CRDT bytes, and no +message expresses a wrap mode. So `truncate` would have changed the TUI +and left the GPU wrapping — the exact disagreement this lane closes. +Specified as an additive variant at v22 (advertised baseline unmoved), +carrying `buffer_id` because the mode is buffer-local, resent on +attach, on config change, **and on buffer switch** — the third being +the one a `FontFacts`-shaped design misses, since font size is global +while wrap mode is per buffer. + +**Q#LL8 (§5d) — "every vertical consumer is local" was false.** The +scroll indicator needs a total: a one-line buffer wrapping to fifty +rows has `total_lines == 1`, so `format_scroll_indicator` returns +`All` while forty-nine rows sit off-screen. §5b.5 is narrowed +accordingly, keeping the distinction that bounds the cost — a *total* +is one lazily-computed number, an *index* is `O(N)` resident. `All`, +`Top` and `Bot` need no aggregate at all; only `NN%` does. + +**Notation (§7).** Revision 14 said `pos_to_display` returns "the +visual row", which reads as redefining `row` — the thing §5b.5 +forbids. Every wrap-point example is now the explicit triple +`{row, sub_row, col}`, and both coordinates at a soft break share the +same `row`, which is the information a redefinition would have +destroyed. + +**Revision 16** — review of `1c9ff6a` found two more, +both in Q#LL8, and both the same shape: a fix that looked complete +because it was correct in one of two places. + +**The GPU has its own indicator** (§5d.3). `format_scroll_indicator` is +**duplicated, not shared** — `src/editor.rs:5509` and +`pmacs-gpu/src/main.rs:10114` — and the GPU passes +`current_line_starts.len()`, a source-line count. So revision 15 would +have fixed the indicator in the TUI and left the GPU reporting `All` +for a one-line wrapped buffer: **this lane's own defect, reproduced by +the section meant to close it.** Both copies keep their signature; what +changes is what the callers pass, so every existing formatter test +stays valid. + +**The lazy total's cache key omitted fold state** (§5d.2). Folds are +per rendered window and can change with no edit, no resize and no mode +change, so all three of revision 15's key components stay put while the +projection moves. Corrected to **(buffer generation, content width, +mode, fold projection)** — keyed on the projection's own `components`, +which is `O(folds)` to compare and **cannot be forgotten**, rather than +a maintained revision counter that can. Same principle as byte-anchoring +and additive `sub_row`: self-validating over maintained. And *content* +width, not window width, because the gutter changes at the line-count +digit boundary. + +**Revision 17** — review of `b95506f` falsified the +premise revision 16 gave the GPU: it said cosmic-text "already knows +each line's visual height". It does not. The GPU shapes **only the +viewport slice** — `rebuild_code_slice` feeds cosmic-text +`current_text[vstart..vend]` because Session S1 found the whole rope +made large-file editing **O(file) per keystroke** +(`pmacs-gpu/src/main.rs:7912`, `:1710`). Its layout cannot produce a +total, and re-shaping the document to get one would reintroduce exactly +that cost — for a status-line readout. + +**So the aggregate is abandoned rather than relocated (§5d.4): `NN%` is +byte-based in both frontends**, with `truncate` keeping today's +visible-line percentage. Computing rows arithmetically would only +*approximate* what cosmic-text actually renders — the same trap Q#LL5 +rejected — and letting the two frontends use different rules would be +this lane's own defect a third time. `All`/`Top`/`Bot` are unaffected: +they are local predicates and stay exact, and they are what users +actually read. + +**This makes revision 16's cache — and its fold-key correction — +unnecessary.** That correction was right for the design as it stood; +the design moved under it. §5d.2 is marked superseded rather than +deleted, so a later reader can tell "the key was fixed" from "there is +no key". §5d.7 adds the **large-file guard witnesses**, because "no +whole-document work happens" must be enforced, not merely intended. + +**Revision 18** — review of `d6b5285` found the +interface contradiction the byte fallback left behind. Revision 16 had +claimed the formatter could keep its signature and change only its +arguments; it cannot. **Every branch of `format_scroll_indicator` +derives from `total_lines`** — including `Bot` via +`view_top + visible >= total_lines`, which with a byte total would +compare **rows against bytes** — and any stand-in small enough to pass +restores the false `All`. "Local predicates" is not something that +signature can express, because it has no parameter for them. + +**Resolved by not asking it to (§5d.5).** `truncate` calls the existing +formatter **untouched**, so its output is byte-identical *by +construction* and every existing formatter test stays valid; `wrap` +calls a new `classify(first_visible, last_visible, byte_pos, byte_len)` +returning `All`/`Top`/`Bot`/`Percent`, which never sees a row count — +so the unit mixing is not avoided but **unrepresentable**. Trying to +serve two genuinely different contracts from one four-count signature +was the mistake; it could only do so by making units implicit, which is +how the contradiction arose. + +**§5d.6 is a new open question**: `pmacs-gpu` depends on +`pmacs-protocol` only, never on the `pmacs` lib, so the duplication is +**structural**. Duplicating the classifier preserves exactly the +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 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 +semantic decision they feed — is a sharper split than "shared +vocabulary", and it adds **no wire message and no version bump**. +Q#LL8 is approved. **The framing is approved; implementation begins.** + +Drafted while GitHub Actions was in a major outage and #220 could not +merge. Nothing here depends on #220 landing; the two lanes touch no +common code. + +The user's report, from daily-driver use: + +> 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. This should also be something that the +> user can configure, whether to wrap or scrollable. + +**The report is accurate where it makes a claim, and it is careful to +limit that claim to the TUI.** In the TUI a line wider than the window +is unreadable past the edge by any means. The GPU is not in that state: +it already wraps, so the text is readable there --- see §1.2, which +also records that revision 1 of this document asserted "in either +frontend" and was wrong to. + +**The cross-frontend defect is not unreadability. It is that neither +behavior was chosen.** The TUI truncates because a cell walk breaks at +`max_cols`; the GPU wraps because a library default was never +overridden. Two accidents that disagree, and no way for the user to +express a preference in either --- which is the part of the report that +applies to both frontends: *"should also be something that the user can +configure."* + +--- + +## 1. What is already built + +Almost nothing, and that is the honest headline. Stage 1 and Stage 2 +both drove machinery that already existed --- Stage 1 honored a flag +with a documented contract, Stage 2 drove a font preference with a +whole wire message behind it. **Stage 3 has no such seam.** It builds a +capability the codebase does not have. + +What exists and helps: + +- **`text_view::render` is the single place a source line becomes + cells** (`src/text_view.rs:211`). One walk, one truncation site --- + **for the grid path only.** See §1.2: that is one of two renderers, + not the renderer. +- **The fold precedent.** Arc 6 already broke the row-to-source-line + identity: `VisibleLineMap` + `Viewport.folds` let row `r` show the + `r`-th *visible* line. The framing for it recorded why folding is not + an overlay --- *"overlays repaint cells, they cannot delete rows"* + (`src/view.rs`). Wrapping is the exact dual: it **adds** rows for one + source line. The same argument forbids it being an overlay, for the + same reason. +- **The config registry already supports buffer-local overrides.** + `Registry::get(name, Option)` consults a per-buffer layer + before global (`src/config_registry.rs:843`), with an explicit note + that there is no ambient current buffer --- a caller wanting + buffer-aware behavior must pass the `BufferId`. So "wrap in prose, + scroll in logs" needs no new machinery. + +What does **not** exist: + +- **No horizontal offset anywhere.** `Viewport` (`src/view.rs:131`) has + `buffer_start`, `buffer_end`, `cell_origin`, `cell_size`, `gutter_w`, + `folds` --- and no column offset. `Window` has `view_top` + (`src/desktop.rs:92`) and no `view_left`. +- **The truncation is one line of code with no alternative path.** + `src/text_view.rs:251`: + ```rust + if col >= max_cols { + break; + } + ``` + 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~~ **WITHDRAWN (revision 20)** + +> **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. + +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 + +**Revision 1 of this document got this wrong and the error inverted the +lane's cost.** It claimed both frontends consume the same `CellGrid`, +so one change in `text_view::render` would reach both for free. That is +false, and `pmacs-gpu` says so in its own words +(`pmacs-gpu/src/main.rs:4502`): + +> The grid variants (`CellDelta`, `Cursor`, `CursorByte`) are +> **ignored** --- pmacs-gpu **lays out locally** and tracks the cursor +> via `PresenceUpdate`. + +The `terminal.rs:772` comment revision 1 cited --- "one run per row, +never wrapped together" --- is the **vterm** path, where a run must +occupy exactly the cells the child gave it. It says nothing about +document text. + +What the GPU actually does with a long line: **it wraps it.** Every +explicit `set_wrap` in `pmacs-gpu` is `Wrap::None` and every one is +chrome --- status, status-left, menu, minibuffer, completion --- or the +terminal run path (`:3930`, `:3940`, `:3956`, `:3966`, `:3978`, +`:5313`, `:5618`, `:8215-8221`). **The document buffer never sets a +wrap mode at all**, so it keeps the one `cosmic-text`'s `Buffer` +constructor installs --- `Wrap::WordOrGlyph` (`buffer.rs:262` in +0.18.2) --- which is what `sync_buffer_dimensions`'s comment assumes: +*"so cosmic-text wraps at the final clip"* (`:4338`). + +It is tested, if incidentally: `wrapped_caret_survives_size_changes` +(`:15853`) puts a 180-character line in a 320px window with the caret +at byte 180 and asserts the caret paints inside the code clip. A +truncating renderer would have that byte off-screen. + +**So the real state of the product is:** + +| | long line | horizontal scroll | +|---|---|---| +| TUI (grid) | **truncated, unreadable** | none | +| GPU (local layout) | **wrapped** --- readable | none | + +This reframes the lane. The user wrote *"Haven't tried this in GUI, but +in TUI, a line that extends off screen cannot be read in full"* --- and +that instinct was exactly right. The GUI half most likely already +works. What is broken there is different: **the wrap is implicit, +unconfigurable, and was never a decision** --- it is cosmic-text's +default leaking through as product behavior. + +**Stage 3 is therefore not "build wrap and scroll." It is "make the two +frontends agree on a mode the user chose."** That is a §16 Semantic +Frontend Architecture concern, and it is a larger lane than revision 1 +implied: the work lands in `text_view::render` *and* in the GPU's local +layout, with a shared setting deciding both. + + +--- + +## 2. The central question: wrap and scroll are not one mechanism + +The user's phrasing --- "either wrap somehow or be scrollable ... +whether to wrap or scrollable" --- reads as one setting with two +values. **Internally they are not two settings on one mechanism; they +are two different mechanisms**, and the framing has to say so before +anything is built. + +- **Horizontal scroll** is a *viewport offset*. Row `r` still shows + exactly one source line. The walk starts at display column + `view_left` instead of 0. The row-to-line relation is untouched. +- **Wrap** is a *row-multiplying line map*. One source line occupies + ceil(width / cols) rows. The row-to-line relation **breaks**, in the + same way folding broke it --- and in the opposite direction. + +They cost very different amounts. Scroll is close to the cheap change +it looks like. Wrap is not. + +### 2.1 What wrap breaks that scroll does not + +`TextView::pos_to_display` (`src/text_view.rs:156`) returns +`DisplayCoord::new(row_idx, col)` where `row_idx` is the **source** +line index (`line_at_offset(pos)`) and `col` is the display width of +the line's prefix. Under wrap +neither half survives: one source line has many rows, and `col` is +width-modulo-cols rather than the prefix width. + +That function is not a detail. Per `src/view.rs`, **cursor placement +and scrolling use the base text view's `pos_to_display` only.** And +`overlay_paint.rs` maps every overlay's display row through `view_top` +plus the fold map (`src/overlay_paint.rs:178`, `:318`). So a new row +mapping that does not go through the same place puts **every overlay +--- diagnostics, highlights, inlay hints --- on the wrong row** for any +buffer containing a wrapped line. + +This is the real cost of wrap, and it is why I am not proposing to +build both at once. + +--- + +## 3. Q#LL1 --- scope: one mechanism or two? **ANSWERED** + +> **Answered 2026-08-06.** Stage 3 ships `wrap` and `truncate`, with +> **`wrap` as the default**; horizontal scroll is Stage 4. The default +> was decided knowingly: no default preserves both frontends, because +> they currently disagree (§1.2), so this one **changes the TUI's +> current behavior** and leaves the GPU's alone. `wrap` wins because it +> is the readable value and the one the reported defect asks for. +> +> Corroborating, though not the reason: **Emacs also wraps by +> default** --- `truncate-lines` is `nil` --- so the choice matches +> what an Emacs-shaped editor's users expect. See §5a for the part of +> that comparison which does *not* transfer. + + +**(a) Horizontal scroll only.** Closes the reported defect --- the line +becomes readable --- at the lowest risk. `view_left` on the window, an +offset in the walk, commands to move it, and cursor-follow. Does not +touch `pos_to_display`'s contract beyond a column shift. + +**(b) Wrap only.** Matches what many users reach for first, but pays +the whole row-mapping cost immediately and puts every overlay's +correctness in the blast radius. + +**(c) Both, in one lane.** Two mechanisms, one review. Against the +project's one-feature-one-branch rule in spirit even if it is one +"feature" in the user's words. + +**(d) Scroll now (Stage 3), wrap as Stage 4.** Ships readability +quickly; leaves the mode setting's second value unimplemented for a +while, which is a discoverability wart --- a setting that names a value +it does not honor is its own coherence defect. + +**Revision 2 changes this recommendation.** Revision 1 recommended (a) +or (d) --- scroll first, wrap later --- on the belief that one renderer +served both frontends. §1.2 shows that is false, and it undermines the +recommendation: with the GPU **already wrapping**, shipping +scroll-only would leave the TUI scrolling while the GUI wraps. The +frontends would still disagree, and the user would still have no say +--- which is the actual complaint. + +**Revised recommendation: (b) wrap first**, as the mode both frontends +can already almost honor, then scroll as Stage 4. + +The reasoning inverts cleanly. Wrap is the expensive one in the grid +renderer and **free in the GPU, where it already happens**; picking it +first means Stage 3 ends with both frontends doing the same declared +thing. Scroll is cheap in the grid renderer and **entirely new in the +GPU** --- which has `scroll_top` but no horizontal counterpart +anywhere (a search for `scroll_left|hscroll|x_offset` in +`pmacs-gpu` returns nothing). + +I still do not recommend (c). But note the cost profile is now the +mirror image of what revision 1 claimed, so please read that +recommendation as withdrawn rather than merely amended. + +### 3.1 The Stage 3 surface, stated + +Wrap-first leaves an obvious hole: a setting with one legal value is +not a setting, and the user asked for a choice. Revision 2 left this +implicit and let a *test* mode stand in for a *product* mode, which is +how §7's witness ended up describing "scroll/truncate" as if they were +one thing. They are not, and scroll is deferred. + +**Decision: Stage 3 ships two values, `wrap` (default) and +`truncate`.** Scroll is Stage 4. + +- **`wrap`** --- the default, because it is the only value that leaves + all text reachable with Stage 3's machinery alone. It is also what + the GPU does today, so the default is not a behavior change there. +- **`truncate`** --- one source line per row, clipped at the edge. This + is exactly what the TUI does today, named and made deliberate, and + made available in the GPU where it currently is not. + +**`truncate` is not a placeholder and not a test-only mode, but it is +incomplete until Stage 4.** The honest description: truncate is the +*mode*, horizontal scroll is the *navigation* that makes the clipped +remainder reachable. "Scrollable" in the user's request decomposes into +exactly those two, and Stage 3 ships the first. Until Stage 4 lands, +selecting `truncate` means accepting that text past the edge cannot be +read --- which is why it must not be the default, and why its +description string has to say so rather than implying a complete +feature. + +The alternative --- ship `wrap` alone with no setting and defer the +whole config surface to Stage 4 --- is defensible and cheaper, and I +rejected it for one reason: it would leave the TUI's *current* +truncation reachable only by not-yet-existing configuration, so the +TUI's existing behavior would become unavailable the moment wrap +landed. Users reading logs want one line per row. Removing that, +even temporarily, is a regression dressed as a fix. + +**If you would rather ship wrap-only and take that regression as +acceptable for one stage, say so and I will cut `truncate` --- but the +framing should not pretend the choice is free either way.** + +--- + +## 4. Q#LL2 --- per-buffer or per-window? **ANSWERED** + +> **Answered 2026-08-06: buffer-local.** Free in the existing registry, +> and it matches Emacs, where `truncate-lines` is a buffer-local +> variable shared by every window on the buffer. A window-local layer +> would be a **third** config layer built for a need nobody has +> reported. +> +> **How the mode reaches the renderer**, which is the part that needed +> deciding: `Viewport` carries the **resolved** mode, exactly as it +> already carries `folds`. The render driver in `editor.rs` holds both +> the registry and the `BufferId`, resolves once per window per frame, +> and `TextView` stays config-agnostic --- respecting the registry's +> "no ambient current buffer" rule (`src/config_registry.rs:837`) +> rather than working around it. +> +> This does put the **mode** per-buffer and the **byte anchor** +> per-window. That split is correct rather than merely tolerable: the +> mode is a property of the content, the anchor is a property of the +> viewport looking at it. + + +The registry supports **buffer-local** overrides today, for free. + +But line display is arguably a **window** property: the same buffer in +a split could reasonably wrap in one pane and truncate in the other. + +**Scoped to Stage 3, this question is only about the mode**, and +buffer-local answers it for free. The follow-on --- that `view_left` +would be unambiguously per-window, since two panes on one buffer must +scroll independently exactly as they already hold independent +`view_top`s (`src/desktop.rs:92`) --- **is Stage 4's, not this +lane's.** + +Recording it here anyway, because the choice made now constrains it: if +Stage 3 makes the mode buffer-local and Stage 4 then needs a +per-window offset, the two halves of one user-facing concept end up +living at different scopes. That is what Emacs effectively does and it +is survivable, but it should be a decision rather than a discovery. +**Q#LL2 asks whether to accept that split now.** + +--- + +## 5. Q#LL3 --- what does the cursor do? + +**Under `wrap`, this is a Stage 3 question and it is not optional.** +`pos_to_display` returns `(source line index, prefix width)`, and +§2.1 shows both halves stop being true once one source line owns +several rows. Cursor placement uses that function exclusively, so a +wrapped buffer with an unrepaired mapping puts the caret on the wrong +row --- in the grid renderer, which is the half that has to be built. +Whatever answers it must also serve `overlay_paint`, or diagnostics +land on the wrong row too. + +**Under `truncate`, the cursor question is trivial** --- one row per +line, the existing mapping holds --- **but only until Stage 4.** Moving +the cursor past the right edge in a truncating view is precisely what +horizontal scroll exists to handle, and Stage 3 has no answer for it: +the caret goes off-screen. That is a real, if minor, sharp edge of +shipping `truncate` without scroll, and §3.1's description string +should own it. + +Deferred to Stage 4, recorded here so it is not rediscovered: whether +explicit horizontal scroll drags the cursor with it (as the wheel does +vertically) or leaves it for the next motion to snap back. The existing +"auto-scroll to keep cursor visible" pass that `scroll_window` +deliberately works around (`src/editor.rs:3624-3628`) is where a +horizontal analog would live, and its comment already records the +hazard --- an unconditional snap-back makes explicit scrolling feel +stuck. + +--- + +## 5a. Q#LL5 --- agreeing on the mode is not agreeing on the wrap **ANSWERED** + +> **Answered 2026-08-06: character wrap in BOTH frontends.** The GPU +> document buffer gets an explicit `Wrap::Glyph` --- its first explicit +> `set_wrap` --- and the grid walk wraps at the character. +> +> **The option analysis changed while deciding, and the change is the +> reason.** "Teach the grid word wrap" looked like the high-effort, +> high-parity option. It is not: cosmic-text performs **Unicode line +> breaking (UAX #14)**, so a grid walk breaking on whitespace would +> diverge on hyphens, CJK and non-breaking spaces. That buys +> *approximate* parity, which is worse than honest divergence because +> it looks unified until it is not. True parity that way requires a +> UAX #14 dependency. +> +> Character wrap in both is the only option that is **true parity, +> cheap, and Emacs-consistent** (Emacs's default wrap is a character +> wrap; word wrap is opt-in `visual-line-mode` / `word-wrap`). It also +> completes the lane's thesis: one declared mode, one declared wrap +> style, two frontends that agree. +> +> **Word wrap becomes a declared third value later** --- +> `ui.line-wrap = "word"` honored by both frontends --- rather than an +> inherited library default that only one frontend has. `ConfigKind::Enum` +> makes adding a choice a clean additive change. +> +> **The regression is real and must be stated where users see it, not +> only here.** GUI users have had word wrap since the GPU frontend +> existed and never opted into losing it. This belongs in the PR +> description and the release notes. + + +Raised by writing §9 and noticing I had put it in "not in scope" as if +it were a detail. It is not. + +The lane's thesis is that two frontends should stop disagreeing. But +choosing `wrap` in both only makes them agree on *whether* to wrap, not +*how*. + +The GPU's value is exact and worth naming: `cosmic-text` 0.18.2 +constructs every `Buffer` with **`Wrap::WordOrGlyph`** +(`buffer.rs:262`) --- word wrap, falling back to glyph wrap for a word +that cannot fit a line by itself. It is not a trait `Default`; the +constructor sets it, which is why the GPU document gets it without ever +asking. + +The natural grid implementation --- keep walking the cell row and +continue on the next --- is a plain **character** wrap. Ship both and +the same buffer at the same width breaks lines in different places in +the two frontends. + +Worth noting for whoever writes the tests: the existing +`wrapped_caret_survives_size_changes` uses `"x".repeat(180)`, a line +with **no word boundary at all**, so it exercises only +`WordOrGlyph`'s glyph fallback. It would pass identically under +`Wrap::Glyph`, and therefore cannot detect the divergence this question +is about. A prose line is needed to see it. + +That is a smaller defect than today's truncate-vs-wrap split, and it +may be an acceptable one. But it is the same *kind* of defect this lane +exists to close, so it should be decided rather than inherited --- +which is precisely the mistake §1.2 documents the GPU already making +once. + +Options: match `WordOrGlyph` in the grid walk (most work, genuine +parity); accept character wrap in the grid and document the divergence; +or set `Wrap::Glyph` on the GPU document buffer so both are character +wraps (cheapest parity --- one line, and it would be the document +buffer's *first* explicit `set_wrap` --- but a visible downgrade for +GUI users who have had word wrap all along without anyone deciding they +should). + +No recommendation yet --- I would rather know Q#LL1's answer first, +since this question only exists if `wrap` ships. + +--- + +## 5b. Q#LL6 --- the visual-row map is the actual design problem + +Revision 7 recorded that `pos_to_display` cannot compute a visual row +from its arguments, and framed that as a signature change. **Review was +right that this understates it: a signature is not a model.** + +### 5b.1 The inverse has the same hole, and it is worse + +`display_to_pos` takes `(&self, buf, coord)` (`src/view.rs:278`) and +opens by treating the row as a **source line index** +(`src/text_view.rs:188`): + +```rust +let row = coord.row as usize; +if row >= self.line_count() { return None; } +``` + +Without width and mode it cannot invert a wrapped visual row at all --- +and unlike the forward direction, it will not fail loudly. It will +return a position from the wrong source line. + +### 5b.2 The vertical stack is in source-line space, end to end + +- **`Window::view_top` is documented as "First buffer line shown at the + top of this window's viewport"** (`src/window.rs:374`). Not a visual + row. +- **Rendering converts it as a source line:** + `window.text_view.line_offset(window.view_top)` (`src/editor.rs:4334`). +- **`move_down` conflates display rows with source lines** + (`src/editor_core.rs:2145`). It reads `coord.row` from + `pos_to_display` --- a **display** row --- then treats it as a source + line: bounding `next_row >= aw.text_view.line_count()` against the + source line count and passing it through `map.next_visible()`. This + is correct today only because display row and source line coincide. + Under `wrap` they diverge, and a one-source-line buffer wrapping to + two visual rows would refuse to move to the second row. + + **Revision 9 called `next_visible`'s argument a "visible-line index", + as though folds introduced a third renumbered space. They do not** + --- see §5b.3. That mistake matters here specifically: if a reader + believed folds renumber, the natural fix to `move_down` would be to + renumber again for wrap, which is precisely the fold-composition + error §5b.3 exists to prevent. + +The same source-row assumption runs through `goal_col` +(`src/window.rs:376`, "sticky display column for vertical motion"), +`visible_rows` and therefore `cursor.page-down` (`src/window.rs:377`), +`scroll_window` (`src/editor.rs:3629`), the gutter's line numbers, and +`overlay_paint`'s row arithmetic (`:178`, `:318`). + +### 5b.3 It must compose with folds, not replace them + +This is the constraint that makes it a design item rather than a +utility --- and getting the existing model right is the first half of +it, because **revision 9 got it wrong in the direction that would cause +the very bug this section prevents.** + +Revision 9 said `VisibleLineMap` "mediates source line to **visible** +line" and drew: + +> ~~source line --(folds)--> visible line --(wrap)--> visual row~~ + +**There is no renumbered visible-line space.** `next_visible(line)` +computes `line + 1` and then jumps past a collapsed component, +returning a **source-line index** (`src/fold_view.rs:223`); so do +`prev_visible`, `visible_head_of` and `clamp_view_top`. + +Folds do not renumber lines. Nor --- a second-order correction, from +review of revision 10 --- do they **restrict the domain**, which is how +this paragraph first put it. `clamp_view_top` *deliberately accepts a +hidden line* and projects it to its visible head; that is the reason it +exists (`src/fold_view.rs:215`), and `text_view::render` relies on it, +noting that "a caller that hands us a hidden start still gets its +head". Calling the domain restricted would imply passing a hidden line +is a caller error. It is the supported case. + +**The first step is a source-index-preserving projection onto a +visible source-line anchor.** Total --- every source line is a legal +input --- idempotent, and staying inside the same index space +throughout. (`visible_rows_between` does return a dense count, but that +is a *distance*, not a coordinate, and nothing indexes with it.) + +That is the same shape as the coordinate rule in §7, one level up: +**identity on canonical inputs; otherwise projection to the contract's +designated canonical representative.** + +"Designated", not "nearest" --- the distinction is the whole content of +the rule. A hidden line maps to its **fold head** even when the next +visible line is closer (`visible_head_of`, not "whichever visible line +is fewest lines away"), and an interior UTF-8 byte maps to its +**codepoint start**, which is not necessarily the nearer boundary. Each +contract names its representative; proximity never selects it. + +The direction is not shared either, which is why the rule has to be +stated in terms of designation rather than of going backward. +`pos_to_display` projects an interior byte **back** to its codepoint +start, while `display_to_pos` rounds a column landing inside a wide +character **forward** to the next codepoint boundary +(`display_to_pos_jumps_over_wide_chars`, +`display_to_pos_inside_tab_rounds_to_next_codepoint`). Two directions, +one rule --- each names its own representative. + +What the two levels genuinely share is the algebra: both are **total** +(every input is legal) and **idempotent** (projecting twice equals +projecting once). That is the property the wrap map must also hold, in +both directions. + +The accurate model, and the one the wrap map must be built against: + +> source line *(projected by folds onto a visible source-line anchor)* +> --(wrap)--> visual row + +So there are exactly **two** coordinate spaces after this lane, not +three: source lines, and visual rows. Wrap is the only renumbering +step, and it consumes a source line the fold map has already projected +onto a visible anchor. + +Why the distinction is load-bearing rather than pedantic: a reader who +believes folds renumber will reach for a second renumbering to layer +wrap on top, ending with a source-to-visible-to-visual chain in which +the middle space has no definition and every fold consumer is +subtly misindexed. A wrap map that instead goes straight from source +line to visual row *without consulting the fold map* bypasses folding +and silently breaks it; one that replaces `VisibleLineMap` +re-implements a merged and reviewed arc. **The map takes a +fold-vouched source line and returns a visual row**, and both +directions have to survive that composition. + +### 5b.4 What Q#LL6 asks --- **ANSWERED** + +> **Answered 2026-08-06**, in discussion. Item 3 (byte-anchored +> `view_top`) is above; two structural decisions are in §5b.5 --- **no +> global map**, and **`DisplayCoord` gains a sub-row rather than +> redefining `row`**. The resize-restore policy §5b.6 demanded is +> **dissolved** rather than answered: a byte anchor makes it +> unnecessary. +> +> **Item 1 --- where it lives: `TextView`, no new type, no cache.** It +> already owns the line offsets and the character walk. Two methods +> taking width and mode. No cache initially: a viewport is ~50 lines, +> so that is at most ~50 single-line layouts per frame, the same order +> as rendering, which already walks every visible line. A per-window +> `(line, width)` cache is a profiling response, not a design premise. +> +> **Item 2 --- signatures.** One `Copy` context value rather than two +> loose parameters: +> +> ```text +> pos_to_display(buf, pos, ctx) -> Option // ctx: { width, mode } +> display_to_pos(buf, coord, ctx) -> Option +> ``` +> +> The asymmetry with the `DisplayCoord` decision (§5b.5) is +> deliberate, and it is the whole audit strategy: +> +> - **The input change is breaking on purpose.** A required parameter +> makes the compiler enumerate every call site, so the §5b.7 audit is +> mechanical rather than a grep. +> - **The output change is additive on purpose.** `sub_row` defaults to +> 0, so a consumer that does not know about wrap stays *correct*, not +> merely *findable*. +> +> Compiler-enforced where enforcement is possible; correct-by-default +> where it is not. + + +1. **Where the map lives and who owns it.** Width is a *window* + property, so a per-window map is the obvious home --- which then + interacts with Q#LL2's buffer-local mode. A per-window map keyed by + a buffer-local mode is coherent but should be stated. +2. **Both directions, explicitly.** Fold-vouched source line to first + visual row, and visual row back to (source line, row-within-line). §7's + witnesses test the second; nothing currently tests the first + because it does not exist. +3. **What `view_top` becomes --- and revision 8 posed this as a false + binary.** It offered "stays a source line" or "becomes a visual + row". **Neither works.** + + A source line **cannot represent a viewport that begins partway + down a wrapped line.** A line taller than the viewport must be + scrollable from its visual row 0 to its visual row 1, and + "convert the source line" always yields row 0 --- so the second + half of a tall line would be unreachable by scrolling. That is not + an edge case; it is the exact situation this lane exists for, since + the motivating buffers are the ones with very long lines. + + A bare visual row fails differently --- see §5b.6. + + **The representation has to be composite: an anchor line plus a + sub-line offset**, composed with folds (§5b.3). This is load-bearing + for cursor visibility, wheel scrolling and paging, not a storage + detail. + + **ANSWERED 2026-08-06: the sub-line component is a BYTE, not a row + index.** `view_top` is the byte offset of the first visible + character (equivalently, anchor line plus byte-within-line). + + A row index is width-dependent and lossy: narrowing then widening + then narrowing again does not return the viewport where it started, + and every width change needs a clamp-or-reset policy. **A byte is + width-independent**, so resize needs no policy at all --- recompute + which row that byte falls on at the new width, exactly and + reversibly. It is the same property that made anchor-line + persistence safe in §5b.6. + + It also composes with the established algebra rather than adding a + rule: an arbitrary byte is not necessarily a row start, so it + projects onto the row start containing it --- total, idempotent, + designated representative, exactly as folds and `pos_to_display` do + (§5b.3). + + And it satisfies the pinned API contract by construction: + `view_top()` returns the line containing that byte; + `set_view_top(n)` sets the byte to line `n`'s start, which *is* + sub-row zero. + + **Precedent, found while answering this:** the GPU already carries a + composite anchor --- `scroll_top` (a source line index into + `current_line_starts`) plus `code_scroll_residual` (a sub-line + offset) --- and `normalize_code_scroll` (`pmacs-gpu/src/main.rs:7955`, + framing Q#F6) already handles reflow pushing that residual across + source lines, by **renormalizing rather than clamping**. So the + composite shape is not novel here. The grid can do better than the + GPU on the sub-component only because it owns its own layout: the + GPU's residual is in pixels because cosmic-text owns layout there, + which is why it needs a renormalization loop that byte-anchoring + does not. +4. **Whether `truncate` is the identity case.** It should be: under + `truncate` the map is the identity and every current behavior holds + unchanged, which is what makes the whole change additive and + testable against today's suite. + +### 5b.5 Two structural decisions taken with Q#LL6 --- **ANSWERED** + +Both from the 2026-08-06 discussion, both load-bearing for cost. + +**1. There is no global logical-to-visual map, and none is needed.** + +A materialized "visual row of every line" prefix sum would be `O(N)` +memory and an `O(N)` rebuild on every width change --- against an M1 +gate that includes `open_100mb_under_200ms`. That is a real perf risk +and §5b.7's "authoritative map" framing invited it. + +No **index** is required, because every *positioning* consumer is +local: rendering walks forward from `view_top` bounded by viewport +height; `move_down`/`move_up` need one step; paging needs +viewport-height rows; the wheel needs *n* rows from `view_top`. Nothing +asks for the absolute visual row of line 40,000, and nothing **indexes** +by one. + +**Revision 14 overstated this as "every vertical consumer is local", +and that is false.** Review of `bd752f2` found the counterexample: the +scroll indicator needs a **total**. See §5d --- and note the +distinction that survives, because it is what keeps the cost bounded: a +*total* is one number, computable lazily and cacheable; a *prefix-sum +index* is `O(N)` resident storage. Stage 3 needs the former and still +does not need the latter. + +So "the map" is two per-line functions --- how many rows this line +occupies at this width, and which row a given byte falls on --- plus +incremental walks. **Layout is needed one line at a time.** + +**2. `DisplayCoord` gains a sub-row; `row` keeps its meaning.** + +`DisplayCoord { row, col }` (`src/view.rs:110`) is core-internal, 53 +references, 39 inside `text_view.rs`'s own tests --- so roughly eight +real external uses. Either approach is tractable in size; they are not +equivalent in risk. + +**Redefining `row` from source line to absolute visual row would +silently break every existing consumer** --- `overlay_paint`'s +`disp.row - view_top` (`:189`, `:321`), `move_down`'s bounds check +(`src/editor_core.rs:2145`) --- with no compile error, which is the +exact failure class this framing has been catching all along. + +Adding a `sub_row` makes it **additive**: `sub_row == 0` under +`truncate` and for every unwrapped line, so existing consumers stay +correct by default and wrap-aware ones opt in explicitly. That is what +bounds the §5b.7 audit: the compiler cannot find these call sites for +us, so the design has to make the untouched ones *right* rather than +merely *findable*. + +### 5b.6 `saveplace` persistence, and why a bare visual row is unsafe + +`view_top` is written to disk. `saveplace` stores one +` ` line per file and parses it with +`^(%d+)%s+(%d+)%s+(.+)$` (`builtin/runtime/saveplace.lua:5`, `:37`), +restoring via `pmacs.editor.set_view_top` (`:76`). Two properties make +this sharper than "a format change": + +- **There is no version marker.** Nothing distinguishes a record + written before this lane from one written after. Redefine what the + second integer means and **every existing record is silently + reinterpreted** --- an old source line 500 becomes visual row 500, + which in any wrapped buffer is a different place entirely. No error, + no migration prompt, just a wrong viewport. +- **The path is the whitespace-split remainder.** So simply appending a + fourth field is not backward-compatible either: an older pmacs + reading a newer file parses the new sub-index as the head of the + path and loses the entry. + +There is a second, independent problem: **a visual row is +width-dependent.** Saved at 120 columns and restored at 80, the same +number denotes a different source location --- and windows legitimately +change width between sessions, which is precisely what QoL Stage 1 was +about. + +**My recommendation, offered as the cheapest correct option rather than +a decision:** persist only the **anchor line**, never the +row-within-line offset. Then the stored value keeps its current +meaning, every existing record stays valid **by construction**, no +migration or version marker is needed, and the persisted number is +width-independent again. The cost is bounded and small: reopening a +file restores to the top of the anchor line rather than partway down +it --- at most one line's height of drift, and only for files closed +mid-wrapped-line. + +**That recommendation is only real if it is an API contract, so state +it as one.** `saveplace` does not touch a field; it calls public Lua +(`builtin/runtime/saveplace.lua:60`, `:76`), and those bindings are +documented today as source lines --- *"view_top(): the active window's +first visible source line"* and *"set_view_top(line): set the first +visible source line"* (`src/lua_bindings/mod.rs:13714`). So the +contract Stage 3 must preserve is: + +- **`pmacs.editor.view_top()` continues to return the source anchor + line**, not a visual row, whatever the internal representation + becomes. +- **`pmacs.editor.set_view_top(n)` sets that anchor with + `row_within_line = 0`.** + +With both held, `saveplace` needs **no change at all** and existing +records keep working --- the compatibility comes from the API contract, +not from `saveplace` being careful. Any future call that needs the +sub-row is a **new** binding, additive, and not what `saveplace` +writes. + +This also decides a question Q#LL6 would otherwise leave open: the +composite `view_top` is an *internal* window representation, and the +Lua surface exposes only its anchor component. Widening the public +getter to return a pair would be the change that breaks records +silently, and it is exactly what "just make `view_top` composite" +invites if the API is not pinned here. + +**Q#LL6 must also settle the resize-restore policy**, which the +composite representation does not escape: when the width changes, a +row-within-line offset may exceed the line's row count at the new +width. Clamp to the last row, or reset to 0? This applies to live +resizes as well as restores, so it is needed regardless of what is +persisted. + +### 5b.7 The cost, restated honestly + +Revision 7 costed this as "~35 `pos_to_display` call sites". **That was +the wrong unit.** The real work is an audit of *both* mapping APIs plus +every place that assumes a display row is a source line --- vertical +motion, paging, wheel scroll, `view_top` handling, gutter numbering, +overlay placement, and the fold interaction above. + +Sizing that audit is itself part of Q#LL6, and it is a strong argument +for `wrap` and `truncate` shipping as one lane with `truncate` as the +identity case: it gives every one of those consumers a mode in which +its current behavior is provably unchanged. + +--- + +## 5c. Q#LL7 --- the GPU needs a wire message, and revision 14 had none + +**Raised in review of `bd752f2`, and it is a hole in the lane's central +claim.** §4 resolves `ui.line-wrap` into `Viewport`, which reaches the +**grid** renderer. The GPU is not a grid consumer (§1.2): it lays out +locally and ignores `CellDelta`. `BufferSnapshot` carries only CRDT +bytes (`pmacs-protocol/src/message.rs:777`), and no `InstanceMessage` +variant expresses a wrap mode. + +So as framed through revision 14, `ui.line-wrap = "truncate"` would +change the TUI and **leave the GPU wrapping** --- the two frontends +still disagreeing, which is the exact defect this lane exists to close. +Q#LL5's "character wrap in both" is likewise unreachable without a +wire: setting `Wrap::Glyph` at GPU startup is not the same as honoring +a mode that can change. + +### 5c.1 The message + +**Additive variant, appended after the current final `InstanceMessage` +variant; `PROTOCOL_VERSION` 21 -> 22; `ADVERTISED_PROTOCOL_VERSION` +stays 20.** This is the path `FontFacts` took at v17 and the panel +shapes took at v21, and the constant's own doc reserves moving the +advertised baseline for changes "that cannot be expressed additively" +--- this one can. + +It carries `buffer_id` alongside the mode. **Not optional: the mode is +buffer-local (§4)**, so "the current mode" is meaningless without +naming the buffer it belongs to, and the GPU tracks +`current_buffer_id` already. + +### 5c.2 Resend semantics --- the part most likely to be got wrong + +The mode must reach the GPU on **all three** of: + +1. **Attach**, for the initially-shown buffer, as part of the same + initial-state burst that establishes font facts. A frontend that + attaches to an existing session must not have to wait for a change + to learn the current mode. +2. **Config change**, via the registry's `on_change` --- for every + attached frontend showing that buffer. +3. **Buffer switch.** This is the one a `FontFacts`-shaped design + misses. Font size is global; **wrap mode is per buffer**, so + switching from a buffer set to `truncate` to one left at `wrap` + changes the effective mode with **no config event at all**. A + design that only listens to `on_change` is silently wrong here, and + would look correct in every single-buffer test. + +### 5c.3 GPU behavior on receipt + +Set `Wrap::Glyph` (mode `wrap`) or `Wrap::None` (mode `truncate`) on +the **document** buffer --- its first explicit `set_wrap` either way +(§1.2) --- then reshape and **renormalize the scroll anchor** through +`normalize_code_scroll` (`pmacs-gpu/src/main.rs:7955`). That path +already exists for exactly this situation: reflow moving the retained +residual across source lines. Changing wrap mode reflows the whole +document, so it is the same event class as a font-size change, and must +reuse that repair rather than reimplement it. + +An out-of-range or unknown mode value is **rejected as a whole +message**, matching `apply_font_facts` rather than clamping --- the +convention Stage 2 followed (`docs/gui-zoom-framing.md`). + +### 5c.4 Older frontends + +A v21-or-older frontend never receives the variant and keeps wrapping. +That is a **documented divergence**, not a silent one: the guarantee +"both frontends agree" holds for peers that negotiated v22, and the +release notes must say so alongside the word-wrap regression (§5a). + +--- + +## 5d. Q#LL8 --- the scroll indicator, which falsifies "everything is local" + +**Raised in review of `bd752f2`.** `format_scroll_indicator` +(`src/editor.rs:5509`) reckons `All`/`Top`/`Bot`/`NN%` from +`total_lines`, fed in visible-line space (`src/editor.rs:4336`, Arc 6 +Q#FD18) so a collapsed remainder correctly reads `All`. + +Under `wrap` that is wrong in a way a user sees immediately. **A +one-line buffer wrapping to fifty screen rows has `total_lines == 1`, +so the very first branch --- `if total_lines <= 1 { return "All" }` --- +reports `All` while forty-nine rows sit below the viewport.** The +indicator claims the whole buffer is on screen when almost none of it +is. + +### 5d.1 The contract + +The indicator is reckoned in **visual rows** whenever the mode is +`wrap`, and in visible lines under `truncate` --- where the two +coincide, so `truncate` remains exactly today's behavior, consistent +with §5b.5's identity-case strategy. + +- `All` --- every visual row of the buffer is on screen. +- `Top` --- the first visual row is on screen and `All` does not hold. +- `Bot` --- the last visual row is on screen and `All` does not hold. +- `NN%` --- **byte position**, not a visual-row ordinal. See §5d.4: + a true row ordinal is unobtainable in the GPU without violating its + large-file design, and approximating it would diverge from what is + actually rendered. + +### 5d.2 What must be computed, and what must not + +**`All` / `Top` / `Bot` need no aggregate.** Each is a local predicate: +is the first visual row on screen (`view_top` byte == first visible +byte), and is the last one (does the forward walk from `view_top` reach +the buffer end within the viewport)? Both fall out of the render walk +that already happens. **Only `NN%` needs a total**, which matters +because `All`/`Top`/`Bot` are the states a user reads most and the +common cases stay `O(viewport)`. + +> **SUPERSEDED by §5d.4 (revision 17).** There is no total and no +> cache: `NN%` is byte-based in both frontends. Everything below was +> correct for the design as it stood in revision 16 and is kept because +> the reasoning still applies to any future aggregate --- and because a +> reader should be able to tell "the key was fixed" from "there is no +> key". Skip to §5d.4 for what is built. + +The total may be computed **lazily and cached** --- but revision 15's +key was wrong, and review of `1c9ff6a` caught it. It said "buffer +generation, width and mode". Two corrections: + +**Fold state must be in the key.** `Viewport.folds` is built **per +rendered window** (`src/view.rs:146`) and a fold can be collapsed or +expanded with **no edit, no width change and no mode change** --- so +all three key components are unchanged while the projection underneath +them is not. Compute `NN%`, collapse a fold, and the stale total is +served for the new projection. + +**Prefer a content-derived key over a maintained one.** +`VisibleLineMap` is `{ components: Vec }` +(`src/fold_view.rs:104`) with no revision field, and +`fold_map_for_window` rebuilds it per call. Two ways to key on it: + +- A revision counter on the fold registry, bumped by every mutation. + Cheap to compare, and it carries a *did-you-remember-to-bump* hazard + on every present and future mutation path --- the same failure shape + as Q#LL7's buffer-switch trigger. +- **The projection's own contents.** `components` holds one entry per + collapsed region, so hashing or comparing it is `O(folds)`, not + `O(N)` --- negligible per frame, and it **cannot be forgotten**, + because the key *is* the thing it guards. + +**Take the second**, for the same reason byte-anchoring beat a row +index (§5b.4) and an additive `sub_row` beat redefining `row` +(§5b.5): a key that derives from the state is self-validating, while +one maintained alongside it is a standing invitation to drift. + +**And it is the CONTENT width, not the window width.** Wrapping happens +in the text area, so the gutter is already subtracted --- and the +gutter's width changes with the line-count digit boundary (9 -> 10, +99 -> 100), which the GPU's `sync_buffer_dimensions` comment already +records for its own shaping (`pmacs-gpu/src/main.rs:4338`). Keying on +window width would serve a stale total across a digit boundary. + +So: **(buffer generation, content width, mode, fold projection)**. + +It composes with folds by counting rows only for lines the fold map +vouches as visible (§5b.3). + +**It must not become a resident prefix-sum index** --- that is the +`O(N)` storage §5b.5 rules out, and the distinction is exactly one +number versus one number per line. + +**The `open_100mb_under_200ms` gate (M1) constrains this.** Computing +total visual rows means laying out every line, so it must not happen on +open, on every frame, or on any path the gate measures --- only on +first `NN%` paint after an invalidation. If that proves too slow on +large buffers, the fallback is to report a **byte-based** percentage +under `wrap` and say so; what is not acceptable is today's silent +`All`. + +### 5d.3 The GPU has its own indicator, and revision 15 missed it + +**Raised in review of `1c9ff6a`.** §5d as written specified only the +TUI path. `format_scroll_indicator` is **duplicated, not shared** --- +`src/editor.rs:5509` and `pmacs-gpu/src/main.rs:10114`, each with its +own tests --- and the GPU calls its copy with +`self.current_line_starts.len()`, a **source-line** count +(`pmacs-gpu/src/main.rs:7199`). + +So a one-line wrapped buffer reports `All` in the GPU too, by an +entirely independent path. **Stage 3 as framed through revision 15 +would have fixed the indicator in one frontend and left it wrong in the +other** --- which is this lane's own defect, reproduced by the lane +meant to close it. + +The GPU's `visible` argument is wrong under `wrap` for the same reason: +`estimated_visible_lines(...)` counts **lines**, and visible *rows* is +what the indicator needs once one line owns several. + +**Revision 16 said both copies could keep their signature and change +only what callers pass. That is not sufficient, and review of +`d6b5285` was right to reject it.** See §5d.5 --- every branch of the +formatter derives from `total_lines`, which revision 17 removed. + +**Revision 16 said the GPU could derive its total locally because +"cosmic-text already knows each line's visual height". That is false**, +and review of `b95506f` caught it. The GPU's cosmic-text buffer holds +**only the viewport slice**: `rebuild_code_slice` shapes +`current_text[vstart..vend]` and nothing else +(`pmacs-gpu/src/main.rs:7912`), because Session S1 found that feeding +the whole rope "made large-file editing **O(file) per keystroke**". +`scroll_top`'s own doc says the same (`:1710`). Its layout cannot yield +total visual rows, nor the cursor's or top's visual-row ordinal. + +Re-shaping the whole document to get them would reintroduce exactly the +cost Session S1 exists to prevent. That is not a tradeoff worth +reopening for a status-line readout. + +### 5d.4 The aggregate is abandoned: byte percentage, both frontends + +**Decision: under `wrap`, `NN%` is computed from BYTE POSITION, in both +frontends. No aggregate, no cache, no invalidation.** Under `truncate`, +both keep today's visible-line percentage unchanged. + +This is the fallback §5d.2 named as a contingency, promoted to the +plan. The reasoning: + +- **The GPU cannot produce a true total** without violating Session S1. +- **Arithmetic would only approximate it.** Rows-per-line could be + computed as `ceil(width / cols)` without shaping --- but cosmic-text + decides the real break points, so the number could disagree with what + is actually on screen. That is the same *approximate parity* trap + Q#LL5 rejected for whitespace wrapping, and it should be rejected + here for the same reason. +- **A divergent choice would be worse than either.** Visual-row `NN%` + in the TUI and byte `NN%` in the GPU means the same buffer shows two + different percentages --- this lane's own defect, for a third time + (§5d.3). One rule in both frontends is the point. +- **`All`/`Top`/`Bot` are unaffected and stay exact**, because they are + local predicates (§5d.2). Those are the states a user actually reads; + `NN%` is a coarse readout, and a byte-based one is honest rather than + wrong. +- Emacs computes its percentage from buffer position too. + +**This makes §5d.2's cache unnecessary, including the fold-key +correction from revision 16.** That correction was right for the design +as it then stood, and the design has since changed underneath it --- +recorded rather than quietly deleted, because "we fixed the key" and +"there is no key" are different states and a later reader should be +able to tell which happened. + +**Known imprecision, stated rather than discovered:** under folds, a +byte percentage counts hidden bytes. Folding is TUI-only today (the GPU +fold stage is unstarted), so this is currently a single-frontend +nuance, and it matches Emacs. It should be revisited **by the GPU +folding lane**, not by this one. + +### 5d.5 The formatter cannot express the new contract --- so it is not asked to + +**The contradiction, stated plainly.** `format_scroll_indicator` +(`src/editor.rs:5509`) derives **every** branch from `total_lines`: + +```rust +if total_lines <= 1 { return "All" } +if visible >= total_lines { return "All" } +if view_top == 0 { return "Top" } +if view_top + visible >= total_lines { return "Bot" } +let pct = (cursor_row + 1) * 100 / total_lines; +``` + +Revision 17 removed the total. So: + +- **Passing byte counts mixes units.** `view_top` and `visible` are + rows; a byte `total_lines` makes `view_top + visible >= total_lines` + compare rows against bytes. It would return plausible strings and be + meaningless. +- **Passing a fake total restores the bug.** Any stand-in that is + `<= 1`, or `<= visible`, returns the false `All` this section exists + to remove. + +"Local predicates" is therefore not something the retained formatter +can evaluate --- it has no parameter for them. + +**Resolution: `truncate` keeps the existing formatter untouched; +`wrap` gets a new classifier.** + +This is §5b.5's identity-case strategy applied to the indicator itself, +and it is stronger than adapting one function to two contracts: + +- **`truncate` calls `format_scroll_indicator` exactly as today**, with + the same arguments in the same units. Byte-identical output is + guaranteed **by construction**, not by a test --- and every existing + formatter test stays valid unchanged, including the GPU's + `format_scroll_indicator(0, 10, 1, 0) == "All"` (`:13091`), which + correctly pins line-space behavior. +- **`wrap` calls a new classifier** that never sees a row total: + + ```text + enum ScrollPosition { All, Top, Bot, Percent(u8) } + + classify(first_visible: bool, // is the buffer's first row on screen? + last_visible: bool, // is the buffer's last row on screen? + byte_pos: u64, // cursor byte + byte_len: u64) -> ScrollPosition + ``` + + `All = first && last`; `Top = first && !last`; `Bot = last && !first`; + otherwise `Percent` from bytes. **No count of rows enters it**, so the + unit mixing above is not merely avoided, it is unrepresentable. + +Attempting one signature for both modes was the actual mistake: the two +contracts genuinely differ, and a shared four-count signature can only +serve them by making units implicit --- which is how this contradiction +arose. + +### 5d.6 Where the classifier lives --- **ANSWERED: `pmacs-protocol`** + +`pmacs-gpu` depends on **`pmacs-protocol` only**, never on the `pmacs` +lib (`pmacs-gpu/Cargo.toml:65`). So `format_scroll_indicator` is +duplicated **structurally**, not by oversight, and a new classifier +faces the same fork: + +- **(a) Duplicate it too.** Matches what is there, adds nothing to any + crate's remit --- and preserves exactly the condition that produced + §5d.3's defect, where one copy was fixed and the other was not. +- **(b) Put it in `pmacs-protocol`.** The only crate both sides + already share. Agreement becomes **structural rather than + maintained** --- the principle that chose byte-anchoring, additive + `sub_row`, and a content-derived cache key. + +**Answered 2026-08-06: (b), in the narrow form.** `ScrollPosition` and +the pure `classify(first_visible, last_visible, byte_pos, byte_len)` +live in `pmacs-protocol`; **string rendering stays in each frontend.** + +The §16 reading that settles it, and it is a better statement of the +split than "shared vocabulary": **each frontend computes its own local +layout facts --- which rows are on screen is a question only it can +answer --- while the shared crate owns the common semantic decision +those facts feed.** That is not presentation moving into the protocol +crate; it is the *decision* being placed where both frontends are +structurally unable to disagree about it, with the rendering left where +it belongs. + +Two properties worth recording because they bound the change: + +- **No wire message and no protocol-version bump.** `classify` is a + pure function over values each side already has. Q#LL7's v22 variant + is unrelated and unaffected. +- **The §5d.3 defect becomes unrepresentable**, rather than guarded by + a reviewer noticing the second copy. A frontend cannot classify + differently, because there is only one classifier. + +### 5d.7 The large-file guard + +Because the whole point is that no whole-document work happens, that +must be a **witness, not an intention**: + +- Painting the indicator on a large buffer with `wrap` active must + perform **no whole-document layout**. In the GPU this is observable + directly --- `view_range` and `shaped_top` must be unchanged by an + indicator paint --- and in the TUI by bounding the lines laid out to + the viewport. +- The existing `open_100mb_under_200ms` gate (M1) must still pass with + `wrap` as the default mode, which is the end-to-end version of the + same claim. + +Without these, the byte-percentage decision is an unenforced comment, +and a later "improvement" to a real row count would silently reintroduce +`O(file)` work. + +**The duplication is itself the hazard worth naming.** Two copies means +two call sites must change, and nothing in the type system connects +them. That is the same shape as Q#LL7's three resend triggers: a +correct fix in one place that looks complete. + +### 5d.8 Verification + +- **The reported case, as a direct witness, IN BOTH FRONTENDS:** one + source line, viewport shorter than its wrapped height, mode `wrap` + --- the indicator must **not** be `All`. This fails against revision + 14's design in the TUI and revision 15's in the GPU, which is what + makes it worth writing first, twice. +- **The large-file guards of §5d.7**, in both frontends --- an + indicator paint must leave the GPU's `view_range` / `shaped_top` + untouched, and must not lay out beyond the viewport in the TUI. +- **`open_100mb_under_200ms` (M1) with `wrap` as the default mode.** +- **A `truncate` output-identity witness against the retained + formatter** (§5d.5): same buffer, same viewport, byte-identical + string. Cheap, and it is the assertion that the identity case is real + rather than asserted. +- **Classifier unit-safety**, which is what the old signature could not + give: `classify` takes two booleans and a byte pair, so a + rows-versus-bytes comparison is unrepresentable. Witness the four + outcomes directly --- `first && last` is `All`, `first && !last` is + `Top`, `last && !first` is `Bot`, neither is `Percent` --- including + the one-line-wrapped case, where `first && !last` must yield `Top` + and **not** `All`. +- The fold witnesses revision 16 asked for are **withdrawn with the + cache they guarded** (§5d.2, §5d.4). What survives from that round is + the `truncate` control below, which still pins the identity case. +- `Top` at the buffer start, `Bot` at the end, `All` only when every + visual row fits --- each with a wrapped line present. +- **A `truncate` control** asserting the indicator is byte-identical to + today's output for the same buffer and viewport. +- A **folded + wrapped** case, since §5b.3's composition applies here + too and the Q#FD18 contract must survive. + +--- + +## 6. Q#LL4 --- `editing.fill-column` **ANSWERED** + +> **Answered 2026-08-06: do not adopt it.** *(Revised 2026-08-07 --- the +> premise was wrong; the answer was not.)* +> +> 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. +> +> **What changed in the deliverables.** One of the two evaporated: +> +> - ~~**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"`. 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. + +--- + +## 7. Verification sketch + +Not final --- it depends on Q#LL1. + +- Unit tests on `text_view::render` at the cell level, **at several + window widths** --- not "at several offsets", which was the same + `view_left` assumption in the very first bullet. A line longer than + `max_cols`; a wide character straddling the wrap/clip column; a tab + expanded across it (the walk's tab path at `src/text_view.rs:254` has + its own `col >= max_cols` break, so wrap has to be taught there too, + not only in the main character path). +- **Wrapped visual-row mapping**, which is Stage 3's version of this. + Revision 3 asked for round trips "at non-zero offset" --- that is a + `view_left` requirement and `view_left` is Stage 4, so the sketch was + quietly re-importing deferred scope. What `wrap` actually needs + witnessed: + - For a source line occupying N visual rows, `pos_to_display` returns + `{ row: source_line, sub_row, col }` --- the **same `row` it + returns today**, plus which visual row *within* that line and the + column within *that* row, rather than the whole prefix width + (`src/text_view.rs:184`). + + **Notation matters here and revision 14 got it wrong.** It said + `pos_to_display` returns "the visual row", which reads as a + redefinition of `row` --- exactly what §5b.5 forbids. Every example + below is therefore written as the explicit triple + `{row, sub_row, col}`; a bare pair anywhere in this section is a + bug in the document, not a shorthand. + - `display_to_pos` inverts it: a click on visual row *k* of a wrapped + line lands in that row's byte range, not the source line's head. + - Round trip is identity for **every valid cursor boundary** in a + wrapped line, walked exhaustively rather than sampled --- the line + is short enough to make that cheap, and sampling is what would miss + the next case. + + **"Every position" would be an impossible invariant, and revision 4 + asked for it.** `pos_to_display` accepts a byte offset *inside* a + multi-byte codepoint and deliberately canonicalizes it: continuation + bytes outside a complete codepoint are trimmed and the codepoint's + own column is the answer (`src/text_view.rs:163-167`), while + `display_to_pos` returns the codepoint **start** + (`src/text_view.rs:185`, and the existing + `display_to_pos_jumps_over_wide_chars` / + `display_to_pos_inside_tab_rounds_to_next_codepoint` pin it). An + interior byte therefore cannot round-trip to itself today, and + demanding it would have made the witness unsatisfiable rather than + discriminating --- the test would have been "fixed" by weakening it, + which is the failure mode this whole sketch is trying to avoid. + + The accurate contract is: **identity on boundaries, projection + elsewhere.** For an interior byte the round trip must land on the + containing codepoint's start, and applying it twice must equal + applying it once. + - **That canonicalization is pre-existing behavior, and this lane + preserves it unless it says otherwise.** It gets its own witness, + separate from the wrap tests, so that "wrap changed the interior-byte + rule" cannot hide inside a wrap failure — or vice versa. If wrap + turns out to need a different rule at a wrap point that also splits a + codepoint, that is a deliberate change with its own Q#, not a quiet + consequence. + - **The wrap point itself**, which is the case worth designing the + test around --- and which revision 5 described in a way that + collapsed two different requirements into one incoherent sentence. + + Take `abcdef` soft-wrapping after `abc`. Buffer positions are + `0=a 1=b 2=c 3=d 4=e 5=f`. **Position 3 is a single source position + with two defensible display coordinates**: + `{row: L, sub_row: k, col: 3}` --- just past the last glyph of + visual row *k* of line *L* --- and `{row: L, sub_row: k+1, col: 0}` + --- just before the first glyph of visual row *k+1* of the **same + source line**. Note both share `row: L`: the wrap point does not + cross a source line, which is precisely why redefining `row` would + have destroyed the information this case turns on. Revision 5 called these "the last + position on row *k* and the first on row *k+1*" and demanded they + "not collide". They are the same position. Nothing can be asserted + about their collision. + + **Decision: the wrap position belongs to column 0 of row *k+1*.** + + Revision 6 justified this by calling the alternative "off-grid", + and parenthetically claimed a hard line end is "within the row". + **Both halves are wrong.** A hard line ending at exactly `max_cols` + gets column `max_cols` --- `pos_to_display` sums the prefix width + and clamps nothing (`src/text_view.rs:184`) --- so it is *also* + just past the last cell, and that is existing, accepted behavior. + Off-gridness therefore does not distinguish the two cases at all. + + Worse, the argument was incoherent on its own terms: + **`pos_to_display` does not know the grid.** Its signature is + `(&self, buf, pos)` (`src/view.rs:271`, `src/text_view.rs:156`) --- + no viewport, no `max_cols`. A function with no notion of the grid + cannot be reasoned about as producing coordinates "off" it. + + The rule that actually decides it, and subsumes both cases: + + > **A position maps to the cell of the glyph that follows it when + > one exists on some row; otherwise to the column just past the + > last glyph.** + + - Soft wrap: position 3 is followed by `d` at + `{row: L, sub_row: k+1, col: 0}`. A + following glyph exists, so that is the answer. There is a genuine + choice here, and this resolves it. + - Hard line end: no glyph follows on any row, so the coordinate is + the column just past the last glyph --- `(k, width)`, **including + `{row: L, sub_row: last, col: max_cols}` when the line fills its + final visual row exactly.** No choice + exists, and **this preserves current behavior unchanged**, which + is the point: the wrap work must not quietly move hard-end + coordinates. + + So the two cases differ because one has an alternative and the other + does not --- not because one is off-grid. + + **A consequence the earlier revisions missed entirely:** under + `wrap`, `pos_to_display` **cannot compute a visual row from its + current arguments.** The wrap width has to reach it. Revision 7 + treated that as a trait signature change and costed it at ~35 call + sites; **that framing was too narrow, and §5b (Q#LL6) replaces it** + --- the inverse has the same hole, and `view_top`, vertical motion, + paging, wheel scroll, gutters and overlays all currently work in + source-line space. Read §5b before costing this. + + Consequences to witness, and they are the discriminating ones: + - `pos_to_display(3)` is `{row: L, sub_row: k+1, col: 0}`, never + `{row: L, sub_row: k, col: 3}`. + - **A hard line end that exactly fills the row still maps to + `{row: L, sub_row: 0, col: max_cols}` with `sub_row` still 0** --- + a control asserting the wrap work left the + existing hard-end coordinate alone, since the soft-wrap rule + superficially resembles a rule that would have moved it. + - `display_to_pos` on the trailing cells of row *k* --- which exist + when a wide character forced an early break and left the row's + last cell blank --- must land on the wrap position, not on the + last glyph's start. This is the case a naive "clamp to row width" + gets wrong. + - **Affinity is explicitly not implemented.** Editors that let + `End` on row *k* and `Home` on row *k+1* sit visually apart at + one buffer position carry an upstream/downstream bit to do it. + Stage 3 carries a single canonical coordinate instead. If that + distinction is wanted later it is a feature with its own state, + not a bug in this mapping --- named here so it is a decision + rather than a discovery. + - **Distinct adjacent codepoints across the break must map + distinctly**, which is the requirement revision 5 was reaching for. + The start of the last codepoint on row *k* (position 2, `c`) and + the start of the first on row *k+1* (position 3, `d`) are two + different positions; they must give `{row: L, sub_row: k, col: 2}` + and `{row: L, sub_row: k+1, col: 0}`, and + the round trip must return each unchanged. + - A `truncate` **control** asserting the mapping is unchanged from + today, so the wrap work cannot silently alter the non-wrapped path. +- If wrap is in scope: an overlay-placement test with a wrapped line + above the overlay's row, which is the regression §2.1 predicts. +- A PTY acceptance test for the user-visible report: with `wrap`, a + line longer than the terminal is readable in full --- following + `full_grid_resync_acceptance.rs`'s content-anchored pattern rather + than any time-based settle. (Not "scrolled past the edge" --- there + is no scrolling in Stage 3. Revision 2 wrote it that way and was + describing Stage 4.) +- **A GPU-side witness that the mode is honored rather than inherited.** + `wrapped_caret_survives_size_changes` (`pmacs-gpu/src/main.rs:15853`) + passes today against a wrap nobody configured, so it cannot + distinguish "honors the setting" from "cosmic-text's default happens + to match." The discriminating case is the other value: **with the + mode set to `truncate`, an overlong line must NOT occupy a second + row.** Without it this lane ships the defect Stage 1 just fixed --- a + declared setting nothing enforces. + + This test is a **negative control for mode enforcement**, and it is + worth being exact about what it does *not* stand for: it is not a + witness for horizontal scroll, and `truncate` is not the user's + "scrollable." Scroll is Stage 4 (§3.1). A reader who takes this test + as evidence that the scroll alternative works would be reading it + 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 + `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. +- **§16 Semantic Frontend Architecture --- this is the lane's primary + coherence citation, per §1.2.** Two frontends currently render the + same buffer's long lines differently, and neither behavior was + chosen: the TUI truncates because the cell walk breaks at `max_cols`, + the GPU wraps because cosmic-text's default was never overridden. + Stage 3 replaces two accidents with one declared mode. +- **No new interaction island.** The mode command goes in the ordinary + command registry and the global keymap. Per Q#Z3's finding in Stage 2, + `keymap_stack::Scope` carries no frontend identity --- and unlike + zoom, that is not a constraint here, because the setting is + frontend-independent by design: both renderers read the same value + and each honors it in its own layout. +- **No background-work attribution.** Nothing async. + +--- + +## 9. Not in scope + +**Horizontal scroll, in full: `view_left` on the window, the commands +that move it, and the cursor-follow pass. That is Stage 4** (§3.1, +§5). Stage 3 ships the `truncate` mode that Stage 4 makes navigable, +and ships it knowing text past the edge is unreachable in the +meantime. + +Reflow/fill commands that *edit* the buffer (`M-q`). Bidi or RTL. A +minimap. Soft-wrap indicators in the gutter --- worth doing, but they +are a gutter-arc concern and would need their own Q#. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 4ee1552..0c71682 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -3913,6 +3913,21 @@ impl State { &mut font_system, Metrics::new(fm.code_font_size(), fm.code_line_height()), ); + // Declare the document's wrap mode instead of inheriting + // cosmic-text's constructor default (`Wrap::WordOrGlyph`). + // + // `Glyph`, not `None`: `ui.line-wrap` defaults to `wrap`, so a + // frontend that has not yet been told anything — or is talking + // to a pre-v22 daemon that never will be — should already be in + // the default mode. What changes versus the inherited default is + // only the break rule, word to character, which is the + // cross-frontend parity this stage buys. + // + // Declaring it is load-bearing rather than tidy: without it the + // document runs on `WordOrGlyph` until some message happens to + // change it, so a frontend talking to a pre-v22 daemon word + // wraps forever while the grid renderer character wraps. + buffer.set_wrap(&mut font_system, Wrap::Glyph); buffer.set_size( &mut font_system, Some(config.width as f32), @@ -5151,6 +5166,10 @@ impl State { self.apply_terminal_frame(frame); None } + InstanceMessage::LineWrapFacts { buffer_id, wrap } => { + self.apply_line_wrap(buffer_id, wrap); + None + } InstanceMessage::PanelFrame(payload) => { // The band changes the DOCUMENT's pixel height, so a panel // that appears or disappears has to reshape the document @@ -7137,7 +7156,14 @@ impl State { /// legacy diagnostic/cursor/scroll suffix. Custom boundaries are one /// base-colored space; the built-in suffix retains its exact two-space /// separators. - fn compose_status_runs(&self) -> Vec<(String, Color)> { + /// `&mut self` because the wrapped branch asks cosmic-text where two + /// bytes actually landed, and laying a line out shapes it. That is + /// the point rather than a wart: the alternative is a per-frame + /// cached `(first_visible, last_visible)` pair, which is a value + /// maintained beside the layout and free to disagree with it — + /// the same shape as the `code_wrap` shadow field this lane already + /// removed once. + fn compose_status_runs(&mut self) -> Vec<(String, Color)> { use std::fmt::Write as _; let base = self.status_right_base_color(); @@ -7196,12 +7222,54 @@ impl State { let _ = write!(readout, "L{}:C{}", line + 1, col + 1); readout.push_str(" "); } - readout.push_str(&format_scroll_indicator( - self.scroll_top, - estimated_visible_lines(self.config.height, self.fm, self.band_inset()), - self.current_line_starts.len(), - cursor_row, - )); + if self.buffer.wrap() == Wrap::None { + readout.push_str(&format_scroll_indicator( + self.scroll_top, + estimated_visible_lines(self.config.height, self.fm, self.band_inset()), + self.current_line_starts.len(), + cursor_row, + )); + } else { + // Wrapping makes the line-space formatter wrong, not merely + // imprecise: it compares `visible` (VISUAL rows that fit) + // against `total_lines` (SOURCE lines), so a document whose + // lines each wrap to three rows reports `Bot` from a third + // of the way down. This is not new in this lane — the GPU + // has always wrapped — but the lane is where it became + // nameable, because `ui.line-wrap` is now what decides which + // formula applies. + // + // The percentage comes from bytes because there is no row + // total to take it from: only the viewport slice is shaped, + // so rows below it were never laid out and counting them + // arithmetically would disagree with the breaks cosmic-text + // actually chose. Same rule as the TUI, from + // `pmacs_protocol::scroll`. + let byte_len = self.current_text.len() as u64; + let byte_pos = self + .own_cursor + .filter(|own| self.current_buffer_id == Some(own.buffer_id)) + .map_or_else( + // No cursor of ours in this buffer: the viewport's + // own top is the honest position, matching the + // `cursor_row = self.scroll_top` fallback above. + || { + self.current_line_starts + .get(self.scroll_top) + .copied() + .unwrap_or(0) + }, + |own| own.byte.min(byte_len), + ); + let first_visible = self.code_byte_painted(0); + let last_visible = self.code_byte_painted(byte_len); + readout.push_str(&render_scroll_position(pmacs_protocol::scroll::classify( + first_visible, + last_visible, + byte_pos, + byte_len, + ))); + } builtins.push((readout, base)); if !runs.is_empty() { @@ -7978,6 +8046,41 @@ impl State { } } + /// Honor a wrap mode for `buffer_id` (protocol v22). + /// + /// The document buffer has never set a wrap mode, so it has been + /// running on cosmic-text's constructor default, + /// `Wrap::WordOrGlyph` — word wrap that nobody chose. This makes the + /// mode explicit in both directions and settles it on + /// **`Wrap::Glyph`**: character wrap is what the grid renderer can + /// implement identically without pulling UAX #14 into it, and it is + /// what Emacs does by default. GUI users lose word wrap; that is a + /// deliberate, documented trade for the two frontends agreeing. + /// + /// Changing wrap reflows the whole document, exactly like a font + /// change, so the retained scroll anchor is repaired through + /// `normalize_code_scroll` rather than left pointing at a row that + /// no longer exists. + fn apply_line_wrap(&mut self, buffer_id: BufferId, wrap: bool) { + // Only the buffer on screen can be reflowed; the mode is + // buffer-local, so a message for anything else is not ours to + // apply. The daemon resends on buffer switch precisely so this + // stays correct rather than needing a per-buffer cache here. + if self.current_buffer_id != Some(buffer_id) { + return; + } + let want = if wrap { Wrap::Glyph } else { Wrap::None }; + // Compare against the BUFFER, not a shadow field. A cached copy + // can disagree with what cosmic-text actually holds — and when + // it does, the short-circuit turns a real mode change into a + // silent no-op. Reading the authority cannot drift from it. + if self.buffer.wrap() == want { + return; + } + self.buffer.set_wrap(&mut self.font_system, want); + self.reshape(); + } + fn reshape(&mut self) { self.rebuild_code_slice(); self.normalize_code_scroll(); @@ -9609,6 +9712,51 @@ impl State { self.minibuffer.is_none() && self.code_caret_rect_in_clip().is_some() } + /// Whether an arbitrary source `byte` is actually painted in the + /// drawable code area — [`Self::code_caret_rect_in_clip`]'s test, + /// generalized off the own cursor. + /// + /// The scroll indicator needs this and nothing weaker. Two cheaper + /// predicates are available and both are wrong: + /// + /// - `view_range.0 == 0` / `view_range.1 == len` describe the + /// **shaped** span, which carries `SCROLL_OVERSCAN` source lines + /// past the window. A slice that merely reaches EOF says nothing + /// about whether EOF is on screen, so `Bot` would latch on early. + /// (This is exactly the guess that broke + /// `extreme_sizes_render_with_contained_popups` when it was tried.) + /// - `scroll_top == 0` ignores `code_scroll_residual`, so scrolling + /// into the middle of a wrapped first line still claims `Top`. + /// + /// Asking where the byte lands answers both, because cosmic-text + /// laid it out: wrapped continuation runs pushed below the band and + /// overscan lines shaped past the bottom both fail the clip. + fn code_byte_painted(&mut self, byte: u64) -> bool { + let (vstart, vend) = self.view_range; + if byte < vstart || byte > vend { + return false; + } + // Deliberately no `vend <= vstart` rejection, though the caret + // and completion-anchor paths both carry one. An empty range is + // not the same as an empty layout: a file ending in a newline + // has a final empty line, and a viewport parked on it shapes + // one real row at `(len, len)`. Rejecting that reported + // "neither end visible" — a percentage — at the exact moment + // the user had scrolled to the bottom. `code_byte_px` already + // returns `None` when nothing is shaped, which is the condition + // that guard was reaching for. + let Some((_x, top, line_height)) = self.code_byte_px(byte) else { + return false; + }; + let y = TEXT_TOP + top; + let bottom = document_text_bottom(self.config.height, self.fm, self.band_inset()); + // Partial overlap counts as painted, the same rule the caret + // clip uses. A first row half-scrolled under the top edge is + // still legible, and a stricter test here would disagree with + // the caret about the very same row. + y < bottom && y + line_height > TEXT_TOP + } + /// Push one rect per visual line whose glyphs overlap the /// slice-relative source byte range `[lo, hi)`, spanning the /// matching projected glyphs' horizontal extent. Each source-line @@ -10108,9 +10256,28 @@ fn minimap_band_contains( && y < MINIMAP_TOP + height } +/// Render a [`pmacs_protocol::scroll::ScrollPosition`] for the status +/// line. The classification is shared with the TUI; only this spelling +/// is local (framing §5d.6), so the two frontends cannot decide +/// differently but each stays free to present it. +fn render_scroll_position(pos: pmacs_protocol::scroll::ScrollPosition) -> String { + use pmacs_protocol::scroll::ScrollPosition; + match pos { + ScrollPosition::All => "All".to_owned(), + ScrollPosition::Top => "Top".to_owned(), + ScrollPosition::Bot => "Bot".to_owned(), + ScrollPosition::Percent(p) => format!("{p}%"), + } +} + /// The TUI mode line's scroll readout, ported verbatim (Q#S1): "All" /// when the buffer fits, "Top"/"Bot" at the extremes, else the cursor /// row as a percentage of the file. +/// +/// Only reached when wrapping is **off**. Under wrapping it is not +/// merely imprecise but wrong — it reckons in source lines while the +/// window holds visual rows — so that path goes through +/// [`render_scroll_position`] instead. fn format_scroll_indicator( view_top: usize, visible: usize, @@ -10484,6 +10651,7 @@ fn debug_apply() -> bool { fn instance_message_label(msg: &InstanceMessage) -> &'static str { match msg { InstanceMessage::CellDelta { .. } => "CellDelta", + InstanceMessage::LineWrapFacts { .. } => "LineWrapFacts", InstanceMessage::Cursor(_) => "Cursor", InstanceMessage::ModeLine(_) => "ModeLine", InstanceMessage::Signal(_) => "Signal", @@ -15847,6 +16015,222 @@ mod tests { /// Acceptance 11 — a caret painted on a wrapped visual run /// survives 16px → 72px → 6px re-wraps, with the normalized + /// A fresh frontend must already be CHARACTER wrapping, not word + /// wrapping. + /// + /// The subtle failure this catches: `apply_line_wrap` short-circuits + /// when the request matches `code_wrap`, so if the field said + /// `Glyph` while the buffer was still on cosmic-text's inherited + /// `WordOrGlyph`, the first `wrap: true` message would be a no-op + /// and the document would keep **word** wrapping — the exact + /// divergence from the grid renderer that framing Q#LL5 exists to + /// close, surviving invisibly. + /// + /// Wrap-versus-truncate cannot see it, because both modes differ + /// from each other either way. Word-versus-character can: with + /// character wrap, spaces are just glyphs, so a spaced line and an + /// unspaced line of the same length occupy the same number of rows. + /// Word wrap breaks early at the spaces and needs more. + #[test] + fn a_fresh_frontend_wraps_by_character_not_by_word() { + let Some(state) = headless_or_skip(320, 400, "hello world\n") else { + return; + }; + assert_eq!( + state.buffer.wrap(), + Wrap::Glyph, + "a frontend told nothing must already be in the DEFAULT mode \ + and wrapping by CHARACTER. Inheriting cosmic-text's \ + WordOrGlyph would word-wrap the document forever against a \ + pre-v22 daemon, diverging from the grid renderer" + ); + } + + /// The discriminating witness framing §7 asked for. + /// + /// `wrapped_caret_survives_size_changes` passes today against a wrap + /// nobody configured — cosmic-text's constructor default — so it + /// cannot tell "honors the setting" from "the default happened to + /// match". The other value is what discriminates: with wrap OFF, an + /// overlong line must NOT occupy a second row. + #[test] + fn the_gpu_honors_an_explicit_non_wrap_mode() { + let long = "x".repeat(400); + let Some(mut state) = headless_or_skip(320, 400, &format!("{long}\nsecond\n")) else { + return; + }; + let bid = BufferId::next(); + state.current_buffer_id = Some(bid); + + state.apply_line_wrap(bid, true); + state.reshape(); + let wrapped_rows = state.buffer.layout_runs().count(); + + state.apply_line_wrap(bid, false); + state.reshape(); + let truncated_rows = state.buffer.layout_runs().count(); + + assert!( + wrapped_rows > truncated_rows, + "wrap must produce more visual rows than truncate \ + (wrapped={wrapped_rows}, truncated={truncated_rows}); equal counts \ + would mean the mode reached nothing" + ); + } + + /// The scroll indicator token: the last builtin in the right-hand + /// status runs, which are joined by a two-space separator. + fn scroll_readout(state: &mut State) -> String { + let runs = state.compose_status_runs(); + let text: String = runs.iter().map(|(text, _)| text.as_str()).collect(); + text.rsplit(" ").next().unwrap_or_default().to_owned() + } + + /// Put `state` in a wrapped, scrolled-to-top state over `text`. + fn wrapped_at_top(state: &mut State, wrap: bool) { + let bid = BufferId::next(); + state.current_buffer_id = Some(bid); + state.apply_line_wrap(bid, wrap); + state.scroll_top = 0; + state.code_scroll_residual = 0.0; + state.reshape(); + } + + /// The defect the shared classifier exists for, in its purest form: + /// **one** source line, wrapping to far more rows than fit. + /// + /// `format_scroll_indicator` returns "All" on its very first branch + /// (`total_lines <= 1`) without consulting anything else — so the + /// mode line claimed the whole buffer was on screen while most of it + /// sat below the window. The truncate control below shows "All" is + /// the right answer *in line space*; it is the space that is wrong. + #[test] + fn a_wrapped_single_line_is_not_all() { + let long = "x".repeat(4000); + let Some(mut state) = headless_or_skip(320, 240, &long) else { + return; + }; + wrapped_at_top(&mut state, true); + assert_eq!( + scroll_readout(&mut state), + "Top", + "one source line wrapping past the window bottom: the first \ + row is on screen and the last is not" + ); + + wrapped_at_top(&mut state, false); + assert_eq!( + scroll_readout(&mut state), + "All", + "control: truncating, this really is one row and all of it \ + is on screen — the line formula is right when it applies" + ); + } + + /// `Bot` must mean the last byte is painted, not that the shaped + /// slice happens to reach EOF. + /// + /// This is the discriminating case for [`State::code_byte_painted`] + /// over the cheaper `view_range.1 == len`: few source lines, each + /// wrapping to many rows, so `SCROLL_OVERSCAN` pulls EOF into the + /// slice while EOF is nowhere near the screen. + #[test] + fn a_slice_that_reaches_eof_is_not_yet_bot() { + let long = "y".repeat(1200); + let text = format!("{long}\n{long}\n{long}\n"); + let Some(mut state) = headless_or_skip(320, 240, &text) else { + return; + }; + wrapped_at_top(&mut state, true); + assert_eq!( + state.view_range.1, + state.current_text.len() as u64, + "precondition: the shaped slice does reach EOF — otherwise \ + this test would pass for the wrong reason" + ); + assert_eq!( + scroll_readout(&mut state), + "Top", + "EOF is shaped but painted far below the band" + ); + + while state.scroll_by_lines(1).is_some() {} + assert_eq!( + scroll_readout(&mut state), + "Bot", + "scrolled to the last source line, EOF is now painted" + ); + } + + /// A file ending in a newline has a final **empty** line, and a + /// viewport parked on it has `view_range == (len, len)`. + /// + /// Named separately because the first version of + /// [`State::code_byte_painted`] rejected an empty range outright — + /// borrowed from the caret path, where it means "nothing shaped". + /// Here it means "one empty row", and rejecting it reported a + /// percentage at the precise moment the user reached the bottom. + #[test] + fn an_empty_final_line_still_counts_as_bot() { + let Some(mut state) = headless_or_skip(320, 240, "alpha\nbeta\n") else { + return; + }; + wrapped_at_top(&mut state, true); + while state.scroll_by_lines(1).is_some() {} + assert_eq!( + state.view_range.0, state.view_range.1, + "precondition: parked on the trailing empty line" + ); + assert_eq!(scroll_readout(&mut state), "Bot"); + } + + /// Scrolling *within* a wrapped first line moves the first byte off + /// screen, and the readout has to notice. + /// + /// `scroll_top == 0` is still true here — which is why the cheap + /// predicate would keep saying `Top` while row one of the document + /// has scrolled away under the top edge. + #[test] + fn a_sub_line_residual_moves_off_top() { + let long = "z".repeat(4000); + let Some(mut state) = headless_or_skip(320, 240, &long) else { + return; + }; + wrapped_at_top(&mut state, true); + assert_eq!(scroll_readout(&mut state), "Top"); + + // Past several wrapped rows, still inside source line 0. + state.code_scroll_residual = state.fm.code_line_height() * 4.0; + state.reshape(); + assert_eq!(state.scroll_top, 0, "still the same source line"); + assert_ne!( + scroll_readout(&mut state), + "Top", + "the document's first row is above the top edge, so `Top` \ + would be a claim about a row that is not painted" + ); + } + + /// A mode for a buffer that is not on screen is not ours to apply. + /// The daemon resends on buffer switch precisely so this holds. + #[test] + fn a_wrap_message_for_another_buffer_is_ignored() { + let Some(mut state) = headless_or_skip(320, 200, "hello\n") else { + return; + }; + let mine = BufferId::next(); + let other = BufferId::next(); + state.current_buffer_id = Some(mine); + state.apply_line_wrap(mine, true); + let after_mine = state.buffer.wrap(); + state.apply_line_wrap(other, false); + assert_eq!( + state.buffer.wrap(), + after_mine, + "another buffer's mode must not reflow this one" + ); + } + /// scroll invariant intact throughout. #[test] #[allow(clippy::float_cmp)] // exact: assigned constants, not computed sums diff --git a/pmacs-protocol/src/lib.rs b/pmacs-protocol/src/lib.rs index 1b14df6..9a0a2dc 100644 --- a/pmacs-protocol/src/lib.rs +++ b/pmacs-protocol/src/lib.rs @@ -41,6 +41,7 @@ pub mod crdt; pub mod ids; pub mod message; pub mod panel; +pub mod scroll; pub mod terminal; pub mod transport; pub mod wire_grid; diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index 73ba142..b4c8e7e 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -1265,6 +1265,40 @@ pub enum InstanceMessage { /// Appended after [`Self::InitialTargetResult`], the final v20 /// variant, so no existing postcard discriminant moves. PanelFrame(crate::panel::PanelFramePayload), + /// Long lines (protocol v22): how the receiving frontend should show + /// a line wider than its viewport, for one buffer. + /// + /// # Why a message exists at all + /// + /// `ui.line-wrap` reaches the grid renderer through the viewport, + /// but `pmacs-gpu` is not a grid consumer — it ignores the + /// `CellDelta` family and lays out locally. Without this, setting + /// the mode would change the TUI and leave the GPU wrapping, + /// which is the cross-frontend disagreement the long-lines stage + /// exists to remove. + /// + /// # Why it names a buffer + /// + /// The setting is **buffer-local**, so "the current mode" is + /// meaningless without saying whose. It follows that the daemon + /// must resend on a **buffer switch** as well as on attach and on + /// config change: font size is global, but wrap mode is not, so + /// moving from a `Truncate` buffer to a `Wrap` one changes the + /// effective mode with no config event at all. A design that only + /// listens for config changes is silently wrong here and looks + /// correct in every single-buffer test. + /// + /// A v21-or-older frontend never receives this and keeps its own + /// behavior; that divergence is documented rather than silent. + /// + /// Appended after [`Self::PanelFrame`], the final v21 variant, so no + /// existing postcard discriminant moves. Daemon-gated `>= 22`. + LineWrapFacts { + /// Buffer the mode applies to. + buffer_id: crate::BufferId, + /// Whether that buffer's long lines wrap. + wrap: bool, + }, } /// One resolved UI face for [`InstanceMessage::ThemeFacts`]: a full @@ -1697,7 +1731,7 @@ pub enum ResourceBody { /// directions: a v20 peer neither receives `PanelFrame` nor is placed in /// a side window, because denying only the events would leave its /// window invisible. -pub const PROTOCOL_VERSION: u32 = 21; +pub const PROTOCOL_VERSION: u32 = 22; /// Protocol version placed in the daemon's server-first [`Hello`]. /// @@ -1865,8 +1899,15 @@ pub fn negotiated_session_version(frontend_offer: u32) -> u32 { /// [`Hello`]. The later capability-activation slice owns moving production /// negotiation to v21 without making existing v20 frontends reject the /// handshake. -pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = - &[6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; +/// +/// Long lines (framing Q#LL7): extended to `[6, ..., 22]` for +/// [`InstanceMessage::LineWrapFacts`]. Additive and daemon-gated, so +/// [`ADVERTISED_PROTOCOL_VERSION`] does not move — a v21 frontend +/// negotiates v21, never receives the variant, and keeps its own +/// behavior. +pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[ + 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, +]; /// T M10.5: predicate for the handshake check. Returns `true` if /// `peer_version` is in [`SUPPORTED_PROTOCOL_VERSIONS`]. diff --git a/pmacs-protocol/src/scroll.rs b/pmacs-protocol/src/scroll.rs new file mode 100644 index 0000000..bab0689 --- /dev/null +++ b/pmacs-protocol/src/scroll.rs @@ -0,0 +1,218 @@ +//! Scroll-position classification, shared by both frontends. +//! +//! # Why this lives in the protocol crate +//! +//! It is not a wire type, and it is deliberately not presentation +//! either. The split it encodes (long-lines framing §5d.6, +//! `COHERENCE.md` §16) is: +//! +//! - **each frontend computes its own local layout facts** — whether +//! the buffer's first or last row is on screen is a question only the +//! frontend that laid the text out can answer; +//! - **the shared crate owns the semantic decision those facts feed** — +//! what `All` / `Top` / `Bot` / `NN%` *mean* is one rule, not two. +//! +//! Rendering the outcome to a string stays in each frontend. +//! +//! `pmacs-gpu` depends on `pmacs-protocol` and never on the `pmacs` +//! lib, so before this module the status readout was **duplicated +//! structurally**: `format_scroll_indicator` exists once in +//! `src/editor.rs` and again in `pmacs-gpu/src/main.rs`, each with its +//! own tests. That is not a tidiness complaint — it produced a real +//! defect during this lane's own review, where a fix landed in one copy +//! and the other kept reporting `All` for a wrapped one-line buffer. +//! With one classifier, a frontend *cannot* classify differently. +//! +//! Adding this needs **no wire message and no protocol-version bump**: +//! [`classify`] is a pure function over values each side already holds. +//! +//! # Why the arguments are booleans and bytes +//! +//! The pre-existing formatter took four counts +//! (`view_top, visible, total_lines, cursor_row`) and derived every +//! branch from `total_lines`. Under line wrapping there is no row total +//! to give it: the GPU shapes only its viewport slice, so it cannot +//! count rows it never laid out, and computing a total by arithmetic +//! would disagree with the break points cosmic-text actually chose. +//! +//! Handing that signature byte counts instead would make +//! `view_top + visible >= total_lines` compare **rows against bytes** — +//! plausible strings, meaningless arithmetic. So the mixing is not +//! merely avoided here, it is **unrepresentable**: the caller supplies +//! two decided predicates and a byte pair, and no count of rows enters +//! this module at all. + +/// Where the viewport sits in its buffer. +/// +/// `Percent` carries whole percent in `0..=100`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ScrollPosition { + /// Every row of the buffer is on screen. + All, + /// The first row is on screen and the last is not. + Top, + /// The last row is on screen and the first is not. + Bot, + /// Neither end is on screen; whole percent through the buffer. + Percent(u8), +} + +/// Classify the viewport's position from local facts. +/// +/// `first_visible` / `last_visible` are the frontend's own answers +/// about its current layout. `byte_pos` is the cursor's byte offset and +/// `byte_len` the buffer's length in bytes; the percentage is taken +/// from those rather than from a row ordinal, because no row total +/// exists (see the module docs). +/// +/// An empty buffer (`byte_len == 0`) cannot have a meaningful +/// percentage, and division would trap. It only ever reaches the +/// `Percent` arm if the caller claims neither end is on screen, which +/// is already contradictory for an empty buffer — so that combination +/// yields `All`, matching what the caller's own facts would have said. +#[must_use] +pub fn classify( + first_visible: bool, + last_visible: bool, + byte_pos: u64, + byte_len: u64, +) -> ScrollPosition { + match (first_visible, last_visible) { + (true, true) => ScrollPosition::All, + (true, false) => ScrollPosition::Top, + (false, true) => ScrollPosition::Bot, + (false, false) => { + if byte_len == 0 { + return ScrollPosition::All; + } + // Widen to u128 before scaling. `saturating_mul` was wrong + // here, not merely inelegant: it *silently undercounts*. + // `u64::MAX * 100` saturates to `u64::MAX`, so a cursor at + // the end of a maximal buffer divided out to 1% — a wrong + // answer that looked safe because it stayed in range. + // + // `u64::MAX * 100` fits in u128 with room to spare, so the + // product is exact and the only clamp left is the genuine + // one below. + let pct = u128::from(byte_pos) * 100 / u128::from(byte_len); + // Clamped for a caller that reports a cursor past the end + // (a stale readout mid-edit): 100%, never above. + ScrollPosition::Percent(u8::try_from(pct.min(100)).unwrap_or(100)) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The four outcomes are decided by the two predicates alone. + #[test] + fn the_two_predicates_decide_the_three_named_states() { + assert_eq!(classify(true, true, 0, 100), ScrollPosition::All); + assert_eq!(classify(true, false, 0, 100), ScrollPosition::Top); + assert_eq!(classify(false, true, 99, 100), ScrollPosition::Bot); + assert!(matches!( + classify(false, false, 50, 100), + ScrollPosition::Percent(_) + )); + } + + /// The case the whole of framing §5d exists for. + /// + /// A one-source-line buffer that wraps to more rows than fit must + /// report `Top`, not `All`. The pre-existing formatter returned + /// `All` here unconditionally — its first branch was + /// `if total_lines <= 1 { return "All" }`, and a wrapped single line + /// still has `total_lines == 1`. + /// + /// This classifier cannot reproduce that bug, because it is never + /// told how many lines there are. + #[test] + fn a_wrapped_single_line_is_top_not_all() { + // One line, first row on screen, last row far below. + assert_eq!(classify(true, false, 0, 4_000), ScrollPosition::Top); + } + + /// Percent comes from bytes, and only when neither end shows. + #[test] + fn percent_is_byte_based_and_only_in_the_middle() { + assert_eq!( + classify(false, false, 250, 1_000), + ScrollPosition::Percent(25) + ); + // Even at a byte position that would read as "the end", an + // explicit `last_visible` wins — the predicate is the fact, the + // percentage is only a readout. + assert_eq!(classify(false, true, 1_000, 1_000), ScrollPosition::Bot); + } + + /// Degenerate inputs stay total: no panic, no wrap, no divide by zero. + #[test] + fn degenerate_inputs_are_total() { + assert_eq!(classify(false, false, 0, 0), ScrollPosition::All); + assert_eq!( + classify(false, false, 10, 5), + ScrollPosition::Percent(100), + "a cursor past the end clamps rather than exceeding 100" + ); + assert_eq!( + classify(false, false, u64::MAX, 1), + ScrollPosition::Percent(100), + "saturating multiply, so a huge offset cannot wrap into a small percent" + ); + } + + /// Percent never leaves `0..=100`, for any input. + /// + /// **In range is not the same as correct**, which is why + /// [`large_byte_counts_stay_accurate`] exists beside this. This + /// sweep passed against a `saturating_mul` that silently reported + /// 1% for a cursor at the end of a maximal buffer — a wrong answer + /// that satisfies every assertion here. + #[test] + fn percent_is_always_in_range() { + for pos in [0_u64, 1, 7, 99, 100, 1_000, u64::MAX / 2, u64::MAX] { + for len in [1_u64, 3, 100, 9_999, u64::MAX] { + if let ScrollPosition::Percent(p) = classify(false, false, pos, len) { + assert!(p <= 100, "pos={pos} len={len} gave {p}%"); + } + } + } + } + + /// The percentage stays *accurate* where `u64` arithmetic would + /// overflow, not merely bounded. + /// + /// `byte_pos * 100` exceeds `u64::MAX` for any position above + /// `u64::MAX / 100`. Saturating there collapses the numerator to a + /// constant, so the quotient stops tracking the position at all: + /// `u64::MAX / u64::MAX` is 1, and the readout said **1%** at the + /// very end of the buffer. + #[test] + fn large_byte_counts_stay_accurate() { + assert_eq!( + classify(false, false, u64::MAX, u64::MAX), + ScrollPosition::Percent(100), + "the end of a maximal buffer is 100%, not 1%" + ); + assert_eq!( + classify(false, false, u64::MAX / 2, u64::MAX), + ScrollPosition::Percent(49), + "halfway through a maximal buffer, floored" + ); + assert_eq!( + classify(false, false, u64::MAX / 4, u64::MAX), + ScrollPosition::Percent(24), + "a quarter through, floored" + ); + // The smallest position whose scaling overflows u64 — the first + // input the old implementation got wrong. + let first_overflowing = u64::MAX / 100 + 1; + assert_eq!( + classify(false, false, first_overflowing, u64::MAX), + ScrollPosition::Percent(1), + "correct by arithmetic here, not by saturation" + ); + } +} 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/daemon.rs b/src/daemon.rs index 62cacc8..fff5e21 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1493,6 +1493,14 @@ fn dispatcher_loop( if !peer_knows_font_facts && matches!(msg, InstanceMessage::FontFacts { .. }) { continue; } + // Long lines — LineWrapFacts gated at v22. A v21 peer + // keeps whatever it does today; the semantic producer + // also skips it, so this is the belt-and-braces half. + if negotiated_protocol_version < 22 + && matches!(msg, InstanceMessage::LineWrapFacts { .. }) + { + continue; + } // Vterm Stage 3 — TerminalFrame gated at v19. A v18 // semantic peer keeps the empty identity snapshot and // no terminal surface; a v18 grid peer is unaffected diff --git a/src/diag.rs b/src/diag.rs index b81a4dc..4ba4d6d 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -757,6 +757,7 @@ fn underline_cols_for_line(line_bytes: &[u8], byte_start: u32, byte_end: u32) -> #[cfg(test)] mod tests { use super::*; + use crate::view::WrapMode; use serde_json::json; fn diag(line: u32, sev: DiagnosticSeverity, msg: &str) -> Diagnostic { @@ -1139,6 +1140,7 @@ mod tests { cell_size: CellSize::new(1, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -1204,6 +1206,7 @@ mod tests { cell_size: CellSize::new(3, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -1280,6 +1283,7 @@ mod tests { cell_size: CellSize::new(3, 8), gutter_w: 2, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -1349,6 +1353,7 @@ mod tests { cell_size: CellSize::new(2, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); diff --git a/src/editor.rs b/src/editor.rs index abd80e8..01cfc08 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -763,6 +763,12 @@ impl EditorState { include_str!("../builtin/runtime/zoom.lua"), ) .expect("load zoom builtin chunk"); + lua_host + .eval( + Some("@pmacs/builtin/runtime/linewrap.lua"), + include_str!("../builtin/runtime/linewrap.lua"), + ) + .expect("load linewrap builtin chunk"); // T M7.11 bundled-package bootstrap. Through M7.10 the REPL // was loaded directly via `eval(include_str!(...))`; the // M7.11 deliverable migrates it to the package system so it @@ -2351,6 +2357,7 @@ impl EditorState { None }; window.last_visible_rows = content.size.rows; + window.last_content_cols = content.size.cols; // A2A-3 / parent 48: the auto-scroll clamp belongs to the // FOCUSED window only. Running it for a passive panel would // move a `view_top` the user is not driving. @@ -3603,7 +3610,9 @@ impl EditorState { let Ok(buf) = reg.get(buffer_id) else { return; }; - core.windows[&win_id].text_view.display_to_pos(buf, target) + core.windows[&win_id] + .text_view + .display_to_pos(buf, target, core.layout_ctx(win_id)) }; if let Some(p) = pos { let aw = core @@ -3662,7 +3671,9 @@ impl EditorState { let reg = registry.borrow(); reg.get(buffer_id).ok().and_then(|buf| { let aw = &core.windows[&win_id]; - let cur = aw.text_view.pos_to_display(buf, aw.cursor)?; + let cur = aw + .text_view + .pos_to_display(buf, aw.cursor, aw.layout_ctx())?; let cur_row = cur.row as usize; let target_row_usize = match folds.as_ref() { Some(map) if scroll_up => map.nth_visible_back(cur_row, view_shift), @@ -3674,7 +3685,11 @@ impl EditorState { }; let target_row = u32::try_from(target_row_usize).ok()?; aw.text_view - .display_to_pos(buf, crate::view::DisplayCoord::new(target_row, cur.col)) + .display_to_pos( + buf, + crate::view::DisplayCoord::new(target_row, cur.col), + aw.layout_ctx(), + ) .or_else(|| aw.text_view.line_offset(target_row_usize)) }) }; @@ -4270,7 +4285,7 @@ fn prepare_window_cursor_visible( ) { let cursor_row = window .text_view - .pos_to_display(buf, window.cursor) + .pos_to_display(buf, window.cursor, window.layout_ctx()) .map_or(0, |d| d.row as usize); match folds { // The logical cursor may sit on a hidden line (a shared fold, or @@ -4348,12 +4363,32 @@ fn paint_window_content( cell_size: crate::cell::CellSize::new(inner_rows, rect.size.cols - gutter_w), gutter_w, folds, + // The resolved mode belongs here beside `folds`, for the same + // reason: the driver holds the registry and the buffer, the view + // holds neither. + // + // It reads `last_wrap` rather than resolving again. The render + // loop already resolved it for this window this frame, and the + // coordinate callers read that same field through + // `Window::layout_ctx` — so a second resolution here could + // disagree with the one the cursor is placed against, which is + // the failure this whole one-resolution arrangement exists to + // prevent. + wrap: window.last_wrap, }; // Composition (T M2.9): base text_view paints first, then the // gutter numbers — before the overlays, so a diagnostic overlay // can draw its severity sign into the gutter's leading column // without the gutter's own blank pass erasing it — then each // overlay in attach order. See [`crate::view::View`]. + // Record the width text actually wrapped at, taken from the + // viewport itself rather than recomputed: a second derivation could + // disagree with the one the renderer used, and the disagreement + // would only show as a cursor on the wrong row. + // Width only: `last_wrap` was resolved before the viewport was + // built and is what the viewport was built FROM, so writing it back + // here would be circular. + window.last_content_cols = viewport.cell_size.cols; window.text_view.render(buf, viewport, grid); if gutter_w > 0 { paint_line_number_gutter(grid, window, &rect, inner_rows, gutter_w, folds, theme); @@ -4366,7 +4401,7 @@ fn paint_window_content( // itself is always visible regardless of overlay activity. let coord = window .text_view - .pos_to_display(buf, window.cursor) + .pos_to_display(buf, window.cursor, window.layout_ctx()) .unwrap_or_default(); // Arc 6 Stage 2 (Q#FD18): All/Top/Bot/% are reckoned in // VISIBLE-line space — a buffer whose remainder is collapsed @@ -4384,7 +4419,30 @@ fn paint_window_content( coord.row as usize, ), }; - let scroll = format_scroll_indicator(ind_top, inner_rows as usize, ind_total, ind_cursor); + let scroll = if window.last_wrap == crate::view::WrapMode::Wrap { + // Under wrapping the line-space formatter is not merely + // imprecise, it is wrong: a one-line buffer wrapping to fifty + // rows has `total_lines == 1`, so its first branch reports + // "All" while forty-nine rows sit below the viewport. + // + // There is no row total to give it instead. The GPU shapes only + // its viewport slice, so it cannot count rows it never laid out, + // and computing a total arithmetically would disagree with the + // break points actually rendered. So `All`/`Top`/`Bot` come from + // LOCAL predicates — which the render walk already knows — and + // the percentage comes from byte position. Both frontends use + // the same rule, from `pmacs_protocol::scroll`. + let first_visible = viewport_buffer_start == 0; + let last_visible = window.text_view.reached_buffer_end(); + render_scroll_position(pmacs_protocol::scroll::classify( + first_visible, + last_visible, + window.cursor, + buf.len(), + )) + } else { + format_scroll_indicator(ind_top, inner_rows as usize, ind_total, ind_cursor) + }; // Lock scoped to the summary computation only: the overlay // renders above include `DiagnosticView`, which takes this // same mutex — holding the guard across the loop deadlocked @@ -4550,6 +4608,13 @@ pub fn paint_frame( // Record viewport height for page motion (cursor.page-down / // cursor.page-up consume this). window.last_visible_rows = inner_rows; + // Resolve the buffer's wrap mode once per window per frame and + // record it here. Every later consumer — the viewport below, + // and the coordinate callers via `Window::layout_ctx` — reads + // this one answer, so nothing re-resolves and two callers + // cannot disagree about one buffer. + window.last_wrap = + crate::lua_bindings::config_line_wrap(state.lua_host.lua(), Some(window.buffer_id)); if inner_rows == 0 || rect.size.cols == 0 { continue; } @@ -4676,7 +4741,9 @@ fn window_cursor_cell( ), None => window.cursor, }; - let disp = window.text_view.pos_to_display(buf, cursor)?; + let disp = window + .text_view + .pos_to_display(buf, cursor, window.layout_ctx())?; let row_offset = match folds { Some(map) => { let top = map.clamp_view_top(window.view_top); @@ -5003,10 +5070,17 @@ fn paint_local_selection( continue; } - let Some(start_coord) = window.text_view.pos_to_display(buf, paint_start) else { + let Some(start_coord) = + window + .text_view + .pos_to_display(buf, paint_start, window.layout_ctx()) + else { continue; }; - let Some(end_coord) = window.text_view.pos_to_display(buf, paint_end) else { + let Some(end_coord) = window + .text_view + .pos_to_display(buf, paint_end, window.layout_ctx()) + else { continue; }; if start_coord.row as usize != display_row || end_coord.row as usize != display_row { @@ -5541,6 +5615,23 @@ fn first_line(s: &str) -> &str { /// `visible` may be 0 in tests that never rendered (so /// `last_visible_rows` was never populated); in that case we fall /// back to cursor-row-based percent without the All/Top/Bot caps. +/// Render a [`pmacs_protocol::scroll::ScrollPosition`] for the status +/// line. +/// +/// The classification is shared with the GPU frontend; only this +/// rendering is per-frontend, which is the split framing §5d.6 settled: +/// each frontend answers its own layout questions, the shared crate owns +/// the decision they feed. +fn render_scroll_position(pos: pmacs_protocol::scroll::ScrollPosition) -> String { + use pmacs_protocol::scroll::ScrollPosition; + match pos { + ScrollPosition::All => "All".to_owned(), + ScrollPosition::Top => "Top".to_owned(), + ScrollPosition::Bot => "Bot".to_owned(), + ScrollPosition::Percent(p) => format!("{p}%"), + } +} + fn format_scroll_indicator( view_top: usize, visible: usize, @@ -5653,6 +5744,7 @@ fn printable_char(seq: &[Chord]) -> Option { #[cfg(test)] mod tests { + use crate::view::WrapMode; // Acceptance home for T M5.4 (FrontendId on input events) — the // `m5_4_*`-prefixed tests verify the FrontendId field threads from // synthetic event construction through `dispatch_key` / @@ -8450,6 +8542,7 @@ mod tests { cell_size: CellSize::new(rect.size.rows, rect.size.cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let mut grid = CellGrid { cells: &mut backing, @@ -8585,6 +8678,7 @@ mod tests { cell_size: CellSize::new(24, 80), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; // Two no-op overlays: probe the dispatch cost only. diff --git a/src/editor_core.rs b/src/editor_core.rs index 6a8ec90..0976756 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -835,6 +835,33 @@ impl EditorCore { crate::fold_view::map_for_window(&self.fold_registry, window) } + /// The layout facts a coordinate mapping needs for `win_id`. + /// + /// **One resolver, so every consumer flips together.** The wrap mode + /// is buffer-local and the registry has no ambient buffer, so the + /// resolution belongs somewhere that holds both — here — rather than + /// at each of the twenty call sites. When `ui.line-wrap` is + /// registered, only this function changes and every caller becomes + /// wrap-aware at once. + /// + /// Width comes from the last render (`Window::last_content_cols`); + /// `0` until the first frame lands, which + /// [`LayoutCtx::wrapping`](crate::view::LayoutCtx::wrapping) already + /// treats as unwrapped. + #[must_use] + pub fn layout_ctx(&self, win_id: WindowId) -> crate::view::LayoutCtx { + self.windows.get(&win_id).map_or_else( + crate::view::LayoutCtx::truncated, + crate::window::Window::layout_ctx, + ) + } + + /// [`Self::layout_ctx`] for the active window. + #[must_use] + pub fn layout_ctx_active(&self) -> crate::view::LayoutCtx { + self.layout_ctx(self.active_window_id()) + } + /// [`Self::fold_map_for_window`] for the active window — the target /// of motion, paging, and the auto-scroll clamp. #[must_use] @@ -2089,6 +2116,7 @@ impl EditorCore { self.normalize_cursor_to_visible(folds.as_ref()); let id = self.active_buffer_id(); let cursor = self.active_window().cursor; + let ctx = self.layout_ctx_active(); let goal_col = self.active_window().goal_col; let result = { let reg = self.registry.borrow(); @@ -2096,7 +2124,7 @@ impl EditorCore { let aw = self.active_window(); let coord = aw .text_view - .pos_to_display(buffer, cursor) + .pos_to_display(buffer, cursor, ctx) .unwrap_or_default(); let from_row = coord.row as usize; if from_row == 0 { @@ -2110,7 +2138,7 @@ impl EditorCore { return; }; let target = DisplayCoord::new(target_row, goal); - let new_pos = aw.text_view.display_to_pos(buffer, target); + let new_pos = aw.text_view.display_to_pos(buffer, target, ctx); (goal, new_pos) }; let (goal, new_pos) = result; @@ -2128,6 +2156,7 @@ impl EditorCore { self.normalize_cursor_to_visible(folds.as_ref()); let id = self.active_buffer_id(); let cursor = self.active_window().cursor; + let ctx = self.layout_ctx_active(); let goal_col = self.active_window().goal_col; let result = { let reg = self.registry.borrow(); @@ -2135,7 +2164,7 @@ impl EditorCore { let aw = self.active_window(); let coord = aw .text_view - .pos_to_display(buffer, cursor) + .pos_to_display(buffer, cursor, ctx) .unwrap_or_default(); let from_row = coord.row as usize; let next_row = folds @@ -2149,7 +2178,7 @@ impl EditorCore { return; }; let target = DisplayCoord::new(next_row, goal); - let new_pos = aw.text_view.display_to_pos(buffer, target); + let new_pos = aw.text_view.display_to_pos(buffer, target, ctx); (goal, new_pos) }; let (goal, new_pos) = result; @@ -2306,6 +2335,7 @@ impl EditorCore { self.normalize_cursor_to_visible(folds.as_ref()); let step = self.page_step(); let cursor = self.active_window().cursor; + let ctx = self.layout_ctx_active(); let view_top = self.active_window().view_top; let id = self.active_buffer_id(); let result = { @@ -2314,7 +2344,7 @@ impl EditorCore { let aw = self.active_window(); let coord = aw .text_view - .pos_to_display(buffer, cursor) + .pos_to_display(buffer, cursor, ctx) .unwrap_or_default(); let max_line = aw.text_view.line_count().saturating_sub(1); let goal_col = aw.goal_col.unwrap_or(coord.col); @@ -2333,7 +2363,7 @@ impl EditorCore { return; }; let target = DisplayCoord::new(target_row, goal_col); - let new_pos = aw.text_view.display_to_pos(buffer, target); + let new_pos = aw.text_view.display_to_pos(buffer, target, ctx); (goal_col, new_pos, new_top) }; let (goal, new_pos, new_top) = result; @@ -2358,6 +2388,7 @@ impl EditorCore { self.normalize_cursor_to_visible(folds.as_ref()); let step = self.page_step(); let cursor = self.active_window().cursor; + let ctx = self.layout_ctx_active(); let view_top = self.active_window().view_top; let id = self.active_buffer_id(); let result = { @@ -2366,7 +2397,7 @@ impl EditorCore { let aw = self.active_window(); let coord = aw .text_view - .pos_to_display(buffer, cursor) + .pos_to_display(buffer, cursor, ctx) .unwrap_or_default(); let goal_col = aw.goal_col.unwrap_or(coord.col); let (target_row, new_top) = match folds.as_ref() { @@ -2383,7 +2414,7 @@ impl EditorCore { return; }; let target = DisplayCoord::new(target_row, goal_col); - let new_pos = aw.text_view.display_to_pos(buffer, target); + let new_pos = aw.text_view.display_to_pos(buffer, target, ctx); (goal_col, new_pos, new_top) }; let (goal, new_pos, new_top) = result; diff --git a/src/frontend.rs b/src/frontend.rs index 275f349..1d04e9f 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -442,6 +442,11 @@ impl Frontend { // grid and negotiates no panel capability, so this cannot // legitimately reach here. | InstanceMessage::PanelFrame(_) + // Long lines: the grid TUI learns its wrap mode through the + // viewport the daemon already resolved for it, so this + // message is for semantic frontends that lay out locally + // and would otherwise never hear the setting at all. + | InstanceMessage::LineWrapFacts { .. } | InstanceMessage::ResourceOffer { .. } // T M11.6 — DispatchIdle is consumed by `attach.rs`'s // optimistic-apply gate; if any reaches this render path diff --git a/src/highlight.rs b/src/highlight.rs index de8ffe0..dfa27c9 100644 --- a/src/highlight.rs +++ b/src/highlight.rs @@ -773,6 +773,7 @@ impl View for LspStyleView { #[cfg(test)] mod tests { use super::*; + use crate::view::WrapMode; #[test] fn theme_lookup_walks_dotted_prefixes() { @@ -1111,6 +1112,7 @@ mod tests { cell_size: CellSize::new(1, 20), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = state.core.borrow().registry.clone(); let reg = registry.borrow(); @@ -1209,6 +1211,7 @@ mod tests { cell_size: CellSize::new(1, 20), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = state.core.borrow().registry.clone(); let reg = registry.borrow(); @@ -1329,6 +1332,7 @@ mod tests { cell_size: CellSize::new(1, 20), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = state.core.borrow().registry.clone(); let reg = registry.borrow(); @@ -1389,6 +1393,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = buf; // keep buf alive hv.render(®istry, viewport, &mut grid); @@ -1446,6 +1451,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = buf; // keep buf alive hv.render(®istry, viewport, &mut grid); @@ -1511,6 +1517,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = buf; hv.render(®istry, viewport, &mut grid); @@ -1578,6 +1585,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; hv.render(&buf, viewport, &mut grid); grid.get(CellCoord::new(0, col)).style @@ -1820,6 +1828,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = buf; hv.render(®istry, viewport, &mut grid); @@ -1883,6 +1892,7 @@ mod tests { cell_size: CellSize::new(rows as u32, cols as u32), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let registry = buf; hv.render(®istry, viewport, &mut grid); 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/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 77bb4c7..b2de320 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -672,6 +672,31 @@ pub fn config_u32(lua: &Lua, name: &str, buffer_id: Option, fallback: } } +/// Resolve `ui.line-wrap` for `buffer_id`. +/// +/// The registry has no ambient current buffer by design, so the caller +/// names the buffer; passing `None` reads the global layer. +/// +/// Falls back to [`WrapMode::Wrap`] rather than `Truncate`, and the +/// direction matters: the fallback covers a bare core whose runtime +/// never loaded the builtin, and it must agree with the setting's own +/// default or a test-constructed editor would silently render in the +/// mode nobody chose — which is the defect this whole stage exists to +/// remove. +#[must_use] +pub fn config_line_wrap(lua: &Lua, buffer_id: Option) -> crate::view::WrapMode { + let Some(registry) = lua.app_data_ref::() else { + return crate::view::WrapMode::Wrap; + }; + let borrowed = registry.borrow(); + match borrowed.get("ui.line-wrap", buffer_id) { + Ok(crate::config_registry::ConfigValue::Str(v)) if v == "truncate" => { + crate::view::WrapMode::Truncate + } + _ => crate::view::WrapMode::Wrap, + } +} + /// Read a `String` setting plus the registry epoch that keys any cache /// built from it (Q#TC4c). /// diff --git a/src/overlay.rs b/src/overlay.rs index cdbce35..80ec62b 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -483,6 +483,7 @@ mod tests { use crate::cell::{Cell, CellSize, Glyph, UnderlineStyle}; use crate::text_view::TextView; use crate::view::Viewport; + use crate::view::WrapMode; fn make_grid(rows: u32, cols: u32) -> Vec { vec![Cell::default(); (rows * cols) as usize] @@ -496,6 +497,7 @@ mod tests { cell_size: CellSize::new(rows, cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, } } diff --git a/src/overlay_paint.rs b/src/overlay_paint.rs index b32195a..7ea555d 100644 --- a/src/overlay_paint.rs +++ b/src/overlay_paint.rs @@ -166,7 +166,10 @@ pub fn paint_other_frontend_overlays( ), None => presence.snapshot.cursor, }; - let Some(disp) = window.text_view.pos_to_display(buf, peer_cursor) else { + let Some(disp) = window + .text_view + .pos_to_display(buf, peer_cursor, window.layout_ctx()) + else { continue; }; // Filter to viewport visible range. `view_top` is the @@ -309,7 +312,10 @@ fn paint_selection_in_window( // straightforward walk is fine. let mut pos = lo; while pos < hi { - let Some(disp) = window.text_view.pos_to_display(buf, pos) else { + let Some(disp) = window + .text_view + .pos_to_display(buf, pos, window.layout_ctx()) + else { break; }; let row_in_window = match folds { diff --git a/src/protocol.rs b/src/protocol.rs index baf2709..1a1c723 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1683,7 +1683,7 @@ mod tests { // --- M5.5a handshake & postcard round-trips --- #[test] - fn protocol_version_is_twenty_one_for_the_bottom_panel_band() { + fn protocol_version_is_twenty_two_for_line_wrap_facts() { // Pin the value: T M10.5 bumped 1→2 (v1.0 wire: CrdtOp / // PresenceUpdate). T M11.1 bumped 2→3 (v1.1 wire: the // SemanticFrame family + FrontendEvent::Viewport). T M11.6 @@ -1728,7 +1728,11 @@ mod tests { // bump that gates in BOTH directions; all four appended after // their enum's final v20 variant, see the placement pins in // `bottom_panel_stage2b_protocol_acceptance`). - assert_eq!(PROTOCOL_VERSION, 21); + // Long lines bumps 21→22 (`InstanceMessage::LineWrapFacts`, + // daemon-gated, appended after the final v21 variant). The + // GPU lays out locally and would otherwise never hear the wrap + // setting; the advertised baseline is deliberately unmoved. + assert_eq!(PROTOCOL_VERSION, 22); } #[test] @@ -1804,18 +1808,18 @@ mod tests { // minibuffer), v13 (`LineNumbers`), v14 (`LineNumberMode`), v15 // (`CompletionPopup`), v16 (`ThemeFacts`), v17 (`FontFacts`), // v18 (`StatuslineSegments`), v19 (the vterm terminal family), - // v20 (semantic initial-target bootstrap), and v21 (the bottom - // panel band) all interoperate. - for accepted in 6..=21 { + // v20 (semantic initial-target bootstrap), v21 (the bottom + // panel band), and v22 (`LineWrapFacts`) all interoperate. + for accepted in 6..=22 { assert!( is_supported_protocol_version(accepted), "v{accepted} must be accepted" ); } - for rejected in [0, 1, 2, 3, 4, 5, 22, u32::MAX] { + for rejected in [0, 1, 2, 3, 4, 5, 23, u32::MAX] { assert!( !is_supported_protocol_version(rejected), - "v{rejected} must be rejected by a v21 binary" + "v{rejected} must be rejected by a v22 binary" ); } } diff --git a/src/search.rs b/src/search.rs index b77d231..a2e84b0 100644 --- a/src/search.rs +++ b/src/search.rs @@ -541,6 +541,7 @@ impl View for SearchView { #[cfg(test)] mod tests { use super::*; + use crate::view::WrapMode; fn r(start: u64, end: u64) -> ByteRange { ByteRange { start, end } @@ -759,6 +760,7 @@ mod tests { cell_size: CellSize::new(1, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -788,6 +790,7 @@ mod tests { cell_size: CellSize::new(1, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid2, ); @@ -830,6 +833,7 @@ mod tests { cell_size: CellSize::new(rows, cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); diff --git a/src/semantic_render.rs b/src/semantic_render.rs index c7f5984..de9f417 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -279,6 +279,12 @@ pub struct SemanticRenderState { /// font state, so this gate has no summary-style companion /// filter. peer_knows_font_facts: bool, + /// `LineWrapFacts` is v22; a v21 peer keeps its own behavior. + peer_knows_line_wrap: bool, + /// Last `(buffer, wrap)` pair sent. Keyed on the PAIR, not the + /// mode: that is what makes a BUFFER SWITCH re-emit without a + /// config event, which a value-keyed cache would miss entirely. + last_line_wrap: Option<(crate::buffer::BufferId, bool)>, /// Whether the peer negotiated protocol >= 18 (Q#SL7). This gates /// callback evaluation in the producer, independently of the daemon's /// write-loop gate. @@ -471,6 +477,7 @@ impl SemanticRenderState { let mut s = Self::new(frontend_id); s.peer_knows_theme_facts = negotiated_protocol_version >= 16; s.peer_knows_font_facts = negotiated_protocol_version >= 17; + s.peer_knows_line_wrap = negotiated_protocol_version >= 22; s.peer_knows_statusline_segments = negotiated_protocol_version >= 18; s.peer_knows_terminal_frames = negotiated_protocol_version >= 19; s.peer_knows_panel_frames = negotiated_protocol_version >= PANEL_MIN_VERSION; @@ -517,6 +524,8 @@ impl SemanticRenderState { last_font_epoch: None, last_font_facts: None, peer_knows_font_facts: true, + peer_knows_line_wrap: true, + last_line_wrap: None, peer_knows_statusline_segments: true, last_statusline: HashMap::new(), diag_line_cache: HashMap::new(), @@ -952,6 +961,7 @@ impl SemanticRenderState { // --- ThemeFacts (UI faces; themes arc Q#TH7, protocol v16) --- out.extend(self.theme_facts_msg(state)); out.extend(self.font_facts_msg(state)); + out.extend(self.line_wrap_msg(state, vp.buffer_id)); // Q#SL6/Q#SL8: face inventory must precede segment text. // Parent acceptance 45: ONE provider invocation supplies both the // primary-document wire segments and the panel mode line, so the @@ -1100,6 +1110,7 @@ impl SemanticRenderState { out.extend(self.minibuffer_prompt_msg(state, buffer_id)); out.extend(self.theme_facts_msg(state)); out.extend(self.font_facts_msg(state)); + out.extend(self.line_wrap_msg(state, buffer_id)); // Q#SL6/Q#SL8: face inventory must precede segment text. // The band rides the terminal path too: a frontend whose DOCUMENT // surface is a full-window terminal can still hold a side window, @@ -2036,6 +2047,38 @@ impl SemanticRenderState { }) } + /// The wrap mode for the buffer this session is showing (v22). + /// + /// `pmacs-gpu` lays out locally and ignores the grid family, so + /// without this it would never hear `ui.line-wrap` at all and would + /// keep wrapping while the TUI truncated — the cross-frontend + /// disagreement the long-lines stage exists to close. + /// + /// Deduped on the `(buffer, wrap)` PAIR. That is not a + /// micro-optimisation: the mode is buffer-local, so switching from a + /// truncating buffer to a wrapping one changes the effective mode + /// with **no config event at all**. A cache keyed on the mode alone + /// would stay silent through exactly that transition — and would + /// look correct in every single-buffer test. + fn line_wrap_msg( + &mut self, + state: &EditorState, + buffer_id: crate::buffer::BufferId, + ) -> Option { + if !self.peer_knows_line_wrap { + return None; + } + let wrap = matches!( + crate::lua_bindings::config_line_wrap(state.lua_host.lua(), Some(buffer_id)), + crate::view::WrapMode::Wrap + ); + if self.last_line_wrap == Some((buffer_id, wrap)) { + return None; + } + self.last_line_wrap = Some((buffer_id, wrap)); + Some(InstanceMessage::LineWrapFacts { buffer_id, wrap }) + } + /// Project the [`Decoration`] set intersecting the declared /// viewport: the session's selection (instance-authoritative, /// byte-native) and LSP diagnostics (line/col → byte, severity → @@ -3738,6 +3781,7 @@ mod tests { | InstanceMessage::LineNumbers { .. } | InstanceMessage::ThemeFacts { .. } | InstanceMessage::FontFacts { .. } + | InstanceMessage::LineWrapFacts { .. } | InstanceMessage::StatuslineSegments { .. } ), "semantic projection emitted an unexpected variant: {m:?}" @@ -3911,13 +3955,17 @@ mod tests { // StatusFacts (Q#S1, cached-compare), the authoritative // ThemeFacts table (Q#TH7 — empty for an unthemed daemon), and // the authoritative FontFacts preference (Q#F5 — all-default), and - // authoritative empty statusline segments (Q#SL8). + // authoritative empty statusline segments (Q#SL8), and the + // buffer's authoritative wrap mode (v22 — a semantic frontend + // lays out locally, so it has to be told on the first frame or + // it never learns the setting at all). let first = s.render_frame(&state); assert_eq!( first.len(), - 7, + 8, "first frame ships StyleSpans + Decorations + FileStyleSummary \ - + StatusFacts + ThemeFacts + FontFacts + StatuslineSegments" + + StatusFacts + ThemeFacts + FontFacts + StatuslineSegments \ + + LineWrapFacts" ); assert_semantic_only(&first); let (style_full, _) = style_segments(&first).expect("StyleSpans present"); @@ -4606,6 +4654,56 @@ mod tests { } } + /// A buffer switch re-emits the wrap mode, with no config event. + /// + /// This is the trigger a `FontFacts`-shaped design misses. Font size + /// is global, so caching it by value is right; wrap mode is + /// **buffer-local**, so a value-keyed cache stays silent when the + /// user moves from one buffer to another with a different mode — + /// and looks perfectly correct in every single-buffer test. + /// + /// Keying the cache on the `(buffer, wrap)` pair is what makes the + /// switch re-emit, so that is what this pins. + #[test] + fn a_buffer_switch_re_emits_the_wrap_mode() { + let state = empty_state(); + let mut sem = local(); + let first = active_buffer(&state); + let second = crate::buffer::BufferId::from_raw(first.raw() + 1); + + let a = sem + .line_wrap_msg(&state, first) + .expect("the first buffer's mode is authoritative"); + assert!(matches!( + a, + InstanceMessage::LineWrapFacts { buffer_id, .. } if buffer_id == first + )); + assert!( + sem.line_wrap_msg(&state, first).is_none(), + "the same pair is suppressed" + ); + + let b = sem + .line_wrap_msg(&state, second) + .expect("a different buffer must be told, even at the same mode"); + assert!(matches!( + b, + InstanceMessage::LineWrapFacts { buffer_id, .. } if buffer_id == second + )); + } + + /// A pre-v22 peer is never sent the variant. + #[test] + fn a_v21_peer_is_not_told_about_wrapping() { + let state = empty_state(); + let mut sem = local(); + sem.peer_knows_line_wrap = false; + assert!( + sem.line_wrap_msg(&state, active_buffer(&state)).is_none(), + "gated at v22; an older peer keeps its own behavior" + ); + } + #[test] fn sibling_of_render_state_reads_same_editor_state() { // The dispatcher selects the projection per session, not per diff --git a/src/text_view.rs b/src/text_view.rs index aae8c06..777e809 100644 --- a/src/text_view.rs +++ b/src/text_view.rs @@ -23,7 +23,7 @@ use crate::buffer::{Buffer, BufferError}; use crate::cell::{Cell, CellCoord, CellGrid, Glyph, Style}; use crate::display_width::{advance_char, valid_prefix_width}; use crate::rope::{Edit, Position}; -use crate::view::{DisplayCoord, View, Viewport}; +use crate::view::{DisplayCoord, LayoutCtx, View, Viewport, WrapMode}; // --------------------------------------------------------------------------- // Tuning @@ -43,6 +43,16 @@ pub const FOLD_ELLIPSIS: char = '…'; /// View that renders a buffer as plain UTF-8 text, one buffer line per row. pub struct TextView { + /// Whether the last [`View::render`] ran out of BUFFER before it ran + /// out of rows — i.e. the buffer's final visual row was on screen. + /// + /// Recorded by the walk rather than recomputed, for the same reason + /// `Window::last_content_cols` is taken from the viewport: under + /// wrapping this cannot be derived from line counts, and a second + /// derivation could disagree with what was actually painted. The + /// scroll indicator reads it as a local predicate, which is what + /// lets `All`/`Top`/`Bot` stay exact with no row total in existence. + reached_buffer_end: bool, /// Byte offsets of each line's first byte. `line_offsets[0] == 0` /// always; `line_offsets.last()` is the start of the final line. /// `line_offsets.len()` equals the number of lines (not the number of @@ -57,11 +67,19 @@ impl TextView { pub fn new(buf: &Buffer) -> Self { let mut v = Self { line_offsets: vec![0], + reached_buffer_end: false, }; v.rebuild_lines_from(buf, 0); v } + /// Whether the last render reached the buffer's end — see + /// [`Self::reached_buffer_end`]. `false` before the first render. + #[must_use] + pub fn reached_buffer_end(&self) -> bool { + self.reached_buffer_end + } + /// Number of lines in the buffer, as understood by this view. #[must_use] pub fn line_count(&self) -> usize { @@ -144,6 +162,281 @@ impl TextView { } out } + + /// Which visual row of `line` holds byte `within` (relative to the + /// line's start), at `max_cols` columns under character wrap. + /// + /// Total: any offset is legal, and one past the line's end lands on + /// its last row. `max_cols == 0` yields row 0 rather than looping. + /// Which visual row of `line` holds byte `within`, discarding the + /// column. Thin wrapper over [`Self::place_of_byte`]. + fn row_of_byte(&self, buf: &Buffer, line: usize, within: u64, max_cols: u32) -> u32 { + self.place_of_byte(buf, line, within, max_cols).0 + } + + /// Where byte `within` (relative to `line`'s start) sits under + /// character wrap, as `(visual row, column)`. + /// + /// Total: any offset is legal, and one past the line's end lands + /// just after its last character. A byte **inside** a multi-byte + /// codepoint yields that codepoint's own place — the same + /// projection `valid_prefix_width` performs on the unwrapped path, + /// so the interior-byte contract is unchanged by wrapping. + fn place_of_byte(&self, buf: &Buffer, line: usize, within: u64, max_cols: u32) -> (u32, u32) { + if max_cols == 0 { + return (0, 0); + } + let bytes = self.read_line_bytes(buf, line); + let Ok(s) = std::str::from_utf8(&bytes) else { + return (0, 0); + }; + let (mut row, mut col, mut seen) = (0u32, 0u32, 0u64); + for ch in s.chars() { + if seen >= within { + break; + } + let (start_row, start_col, end_row, end_col) = + advance_wrapped(row, col, ch, max_cols, true); + seen += ch.len_utf8() as u64; + if seen > within { + // `within` fell inside this character: project to the + // character's own start, which is where it is drawn. + return (start_row, start_col); + } + row = end_row; + col = end_col; + } + // A position that lands exactly on a row boundary belongs to + // column 0 of the NEXT row, not one past the end of the last + // one (framing §7: the wrap position is owned downstream). The + // downstream cell always exists; `(row, max_cols)` does not. + if col >= max_cols { + (row.saturating_add(1), 0) + } else { + (row, col) + } + } + + /// Byte offset (relative to `line`'s start) at visual row `sub_row`, + /// column `col`, under character wrap — the inverse of + /// [`Self::place_of_byte`]. + /// + /// Rounds forward to the next character boundary when the column + /// lands inside a wide glyph, matching the unwrapped + /// `display_to_pos`. A row past the line's height clamps to the + /// line's end. + fn byte_at_place( + &self, + buf: &Buffer, + line: usize, + sub_row: u32, + col: u32, + max_cols: u32, + ) -> u64 { + let bytes = self.read_line_bytes(buf, line); + let Ok(s) = std::str::from_utf8(&bytes) else { + return 0; + }; + if max_cols == 0 { + return 0; + } + let (mut row, mut c, mut walked) = (0u32, 0u32, 0u64); + for ch in s.chars() { + let (start_row, start_col, end_row, end_col) = + advance_wrapped(row, c, ch, max_cols, true); + if start_row > sub_row || (start_row == sub_row && start_col >= col) { + return walked; + } + walked += ch.len_utf8() as u64; + row = end_row; + c = end_col; + } + walked + } + + /// Paint one source line and report how many grid rows it used. + /// + /// `first_row` is where the line begins in the viewport; `skip_rows` + /// drops that many of the line's own leading visual rows, which is + /// non-zero only for the first line when the byte anchor sits partway + /// down it (framing Q#LL6). + /// + /// Under [`WrapMode::Truncate`] this returns 1 and walks exactly as + /// the pre-wrap renderer did — the identity case the staging rests on. + fn paint_line( + &self, + buf: &Buffer, + line: usize, + viewport: Viewport<'_>, + cells: &mut CellGrid<'_>, + place: LinePlacement, + ) -> u32 { + let LinePlacement { + first_row, + skip_rows, + is_fold_head, + } = place; + let max_rows = viewport.cell_size.rows; + let max_cols = viewport.cell_size.cols; + let origin = viewport.cell_origin; + let wrapping = viewport.wrap == WrapMode::Wrap; + + // A zero-width content area has no cell to paint into. Bail + // before the walk rather than inside it: under `Wrap` the first + // `col >= max_cols` test is true immediately, so the walk would + // advance a row and then index column 0 of a zero-width grid. + // Reachable whenever the gutter consumes the window's width. + if max_cols == 0 { + return 1; + } + let line_bytes = self.read_line_bytes(buf, line); + let Ok(s) = std::str::from_utf8(&line_bytes) else { + return 1; + }; + + // `sub_row` counts this line's own visual rows. The grid row is + // derived and is `None` while still skipping, or once past the + // viewport's bottom. + let grid_row = |sub: u32| -> Option { + let r = first_row.checked_add(sub.checked_sub(skip_rows)?)?; + (r < max_rows).then_some(origin.row + r) + }; + let put = |cells: &mut CellGrid<'_>, sub: u32, col: u32, glyph: Glyph| { + if let Some(row) = grid_row(sub) { + let cell = cells.at(CellCoord::new(row, origin.col + col)); + cell.glyph = glyph; + cell.style = Style::default(); + cell.attachment = None; + } + }; + + let (mut sub_row, mut col) = (0u32, 0u32); + for ch in s.chars() { + if !wrapping && col >= max_cols { + break; + } + let (start_row, start_col, end_row, end_col) = + advance_wrapped(sub_row, col, ch, max_cols, wrapping); + if start_row >= skip_rows && grid_row(start_row).is_none() { + sub_row = start_row; + break; + } + if ch == '\t' { + for c in start_col..end_col { + put(cells, start_row, c, Glyph::Char(' ')); + } + } else if end_col > start_col || start_row > sub_row { + put(cells, start_row, start_col, Glyph::Char(ch)); + if end_col.saturating_sub(start_col) == 2 && start_col + 1 < max_cols { + put(cells, start_row, start_col + 1, Glyph::Continuation); + } + } + sub_row = end_row; + col = end_col; + } + + // The head of a collapsed region carries a trailing ellipsis in + // the CONTENT area (Q#FD13/FD20): the authoritative, + // layout-neutral fold indicator, present in every gutter state, + // clipped like any long line. + if is_fold_head { + for marker in [' ', FOLD_ELLIPSIS] { + if col >= max_cols { + if !wrapping { + break; + } + sub_row += 1; + col = 0; + if sub_row >= skip_rows && grid_row(sub_row).is_none() { + break; + } + } + put(cells, sub_row, col, Glyph::Char(marker)); + col += 1; + } + } + + sub_row.saturating_sub(skip_rows) + 1 + } +} + +/// Where a line goes in the viewport, for [`TextView::paint_line`]. +#[derive(Copy, Clone, Debug)] +struct LinePlacement { + /// Grid row (viewport-relative) where this line begins. + first_row: u32, + /// Leading visual rows of the line to drop, non-zero only for the + /// first line when the byte anchor sits partway down it. + skip_rows: u32, + /// Whether the line heads a collapsed region and owes an ellipsis. + is_fold_head: bool, +} + +/// Where a character is drawn, and where it leaves the cursor, under +/// character wrap. +/// +/// **This is the wrap rule and it exists exactly once.** Both +/// [`TextView::row_of_byte`] and [`TextView::paint_line`] go through it, +/// because they must agree perfectly: the first decides which visual row +/// the viewport's byte anchor sits on, the second decides which row the +/// text is drawn on. Two copies that drifted by one row would scroll the +/// buffer to a position it does not render — a defect with no local +/// symptom, and the exact shape this lane keeps finding. +/// +/// Returns `(start_row, start_col, end_row, end_col)`: where the +/// character itself goes (it may already have moved to the next row), +/// and where the following character resumes. +/// +/// With `wrapping == false` the row never advances and the column +/// arithmetic is the pre-wrap walk's own, unchanged. +fn advance_wrapped( + row: u32, + col: u32, + ch: char, + max_cols: u32, + wrapping: bool, +) -> (u32, u32, u32, u32) { + // The break belongs to the character that could not fit, so it is + // taken before drawing rather than after the previous glyph. + let (row, col) = if wrapping && col >= max_cols { + (row.saturating_add(1), 0) + } else { + (row, col) + }; + if ch == '\t' { + // A tab fills to the row's end and stops; it never spans a wrap. + // Column 0 of the next row is itself a tab stop, so alignment + // survives the break rather than being approximated — carrying + // the remaining pad across would put the next character at a + // column the tab-stop arithmetic never chose. + return (row, col, row, advance_char(col, ch).min(max_cols)); + } + let width = advance_char(col, ch) - col; + if width == 0 { + // Combining mark or other zero-width control: M1.5 skips; M2+ + // will attach it to the previous cell as `Glyph::Cluster`. + return (row, col, row, col); + } + // `max_cols >= 2` is the whole of the narrow-viewport policy: a + // double-width glyph moves to the next row only when the next row + // could actually hold it. At one column it never can, so moving + // would insert a blank row before every wide character and paint it + // clipped anyway — a single CJK glyph would render on row 1 with + // row 0 left empty. Below two columns a wide glyph is clipped in + // place, which is what `Truncate` does at the edge for the same + // reason: there is no better row to move it to. + if wrapping && width == 2 && max_cols >= 2 && col + 1 >= max_cols { + // A double-width glyph with a single cell left moves to the next + // row whole rather than being split across the break. + // + // `Truncate` keeps its existing behavior instead — lead cell + // painted, continuation omitted. That is arguably worse, and + // changing it is not this lane's to make: `Truncate` must stay + // byte-identical. + let r = row.saturating_add(1); + return (r, 0, r, 2.min(max_cols)); + } + (row, col, row, col + width) } impl View for TextView { @@ -153,12 +446,17 @@ impl View for TextView { Ok(()) } - fn pos_to_display(&self, buf: &Buffer, pos: Position) -> Option { + fn pos_to_display(&self, buf: &Buffer, pos: Position, ctx: LayoutCtx) -> Option { if pos > buf.len() { return None; } let row_idx = self.line_at_offset(pos); let line_start = self.line_offsets[row_idx]; + if ctx.wrapping() { + let (sub_row, col) = self.place_of_byte(buf, row_idx, pos - line_start, ctx.cols); + return Some(DisplayCoord::wrapped(row_idx as u32, sub_row, col)); + } + // Everything below is the pre-wrap path, unchanged. // Slice [line_start, pos) and sum the display widths of any complete // codepoints inside. Bytes that look like UTF-8 continuation bytes @@ -185,12 +483,22 @@ impl View for TextView { Some(DisplayCoord::new(row_idx as u32, col)) } - fn display_to_pos(&self, buf: &Buffer, coord: DisplayCoord) -> Option { + fn display_to_pos( + &self, + buf: &Buffer, + coord: DisplayCoord, + ctx: LayoutCtx, + ) -> Option { let row = coord.row as usize; if row >= self.line_count() { return None; } let line_start = self.line_offsets[row]; + if ctx.wrapping() { + let within = self.byte_at_place(buf, row, coord.sub_row, coord.col, ctx.cols); + return Some(line_start + within); + } + // Everything below is the pre-wrap path, unchanged. let line_bytes = self.read_line_bytes(buf, row); let s = std::str::from_utf8(&line_bytes).ok()?; @@ -226,83 +534,66 @@ impl View for TextView { let max_cols = viewport.cell_size.cols; let origin = viewport.cell_origin; - let mut line = start_line; + // Under `Wrap` one source line can own several rows, so the row + // walk is no longer the line walk and `row_offset` is carried + // rather than iterated. Clearing moves up front for the same + // reason — a row's occupant is not known until the line reaching + // it has been laid out — and every row is still blanked exactly + // once, as before. for row_offset in 0..max_rows { let cell_row = origin.row + row_offset; - - // Always clear the visible row first so previous content does - // not bleed when the buffer shrinks past this row. for col in 0..max_cols { *cells.at(CellCoord::new(cell_row, origin.col + col)) = Cell::default(); } - if line >= self.line_count() { - continue; - } + } + + // Visual rows of the first line to skip. `buffer_start` may sit + // partway down a wrapped line — the reason the anchor is a byte + // and not a row index (framing Q#LL6). Always 0 under `Truncate`, + // where a line owns exactly one row. + let mut skip_rows = if viewport.wrap == WrapMode::Wrap { + let line_start = self.line_offsets.get(start_line).copied().unwrap_or(0); + self.row_of_byte( + buf, + start_line, + viewport.buffer_start.saturating_sub(line_start), + max_cols, + ) + } else { + 0 + }; + + let mut row_offset: u32 = 0; + let mut line = start_line; + while row_offset < max_rows && line < self.line_count() { let this_line = line; line = folds.map_or(this_line + 1, |m| m.next_visible(this_line)); - - let line_bytes = self.read_line_bytes(buf, this_line); - let Ok(s) = std::str::from_utf8(&line_bytes) else { - continue; - }; - - let mut col: u32 = 0; - for ch in s.chars() { - if col >= max_cols { - break; - } - if ch == '\t' { - // Expand to the next protocol-wide tab stop with spaces. - let pad = advance_char(col, ch) - col; - for _ in 0..pad { - if col >= max_cols { - break; - } - let cell = cells.at(CellCoord::new(cell_row, origin.col + col)); - cell.glyph = Glyph::Char(' '); - cell.style = Style::default(); - cell.attachment = None; - col += 1; - } - continue; - } - let width = advance_char(col, ch) - col; - if width == 0 { - // Combining mark or other zero-width control: M1.5 - // skips; M2+ will attach to the previous cell as - // Glyph::Cluster. - continue; - } - let cell = cells.at(CellCoord::new(cell_row, origin.col + col)); - cell.glyph = Glyph::Char(ch); - cell.style = Style::default(); - cell.attachment = None; - if width == 2 && col + 1 < max_cols { - let cont = cells.at(CellCoord::new(cell_row, origin.col + col + 1)); - cont.glyph = Glyph::Continuation; - cont.style = Style::default(); - cont.attachment = None; - } - col += width; - } - - // The head of a collapsed region carries a trailing ellipsis - // in the CONTENT area (Q#FD13/FD20): the authoritative, - // layout-neutral fold indicator, present in every gutter - // state, clipped like any long line. - if folds.is_some_and(|m| m.is_head(this_line)) { - for marker in [' ', FOLD_ELLIPSIS] { - if col >= max_cols { - break; - } - let cell = cells.at(CellCoord::new(cell_row, origin.col + col)); - cell.glyph = Glyph::Char(marker); - cell.style = Style::default(); - cell.attachment = None; - col += 1; - } - } + let used = self.paint_line( + buf, + this_line, + viewport, + cells, + LinePlacement { + first_row: row_offset, + skip_rows, + is_fold_head: folds.is_some_and(|m| m.is_head(this_line)), + }, + ); + skip_rows = 0; + // A line always advances the row cursor, even when it painted + // nothing (invalid UTF-8, an empty line): otherwise the walk + // would re-enter the same grid row forever. + row_offset += used.max(1); } + // Ran out of buffer before running out of rows. + // + // BOTH halves are needed. `line >= line_count` alone is true + // whenever the last line was *started*, which under wrapping + // happens while its remaining rows sit below the viewport — a + // fifty-row line begun on the last visible row would report the + // buffer end as on screen. `row_offset <= max_rows` is what says + // the rows it needed actually fit. + self.reached_buffer_end = line >= self.line_count() && row_offset <= max_rows; } } @@ -316,6 +607,7 @@ mod tests { use crate::buffer::{BufferId, EditOp}; use crate::cell::CellSize; use crate::rope::Range; + use crate::view::WrapMode; use proptest::prelude::*; fn buf_with(content: &[u8]) -> Buffer { @@ -437,24 +729,54 @@ mod tests { #[test] fn ascii_pos_to_display_basic() { let (buf, view) = attached(b"hello\nworld"); - assert_eq!(view.pos_to_display(&buf, 0), Some(DisplayCoord::new(0, 0))); - assert_eq!(view.pos_to_display(&buf, 5), Some(DisplayCoord::new(0, 5))); - assert_eq!(view.pos_to_display(&buf, 6), Some(DisplayCoord::new(1, 0))); - assert_eq!(view.pos_to_display(&buf, 11), Some(DisplayCoord::new(1, 5))); - assert_eq!(view.pos_to_display(&buf, 12), None); + assert_eq!( + view.pos_to_display(&buf, 0, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 0)) + ); + assert_eq!( + view.pos_to_display(&buf, 5, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 5)) + ); + assert_eq!( + view.pos_to_display(&buf, 6, LayoutCtx::truncated()), + Some(DisplayCoord::new(1, 0)) + ); + assert_eq!( + view.pos_to_display(&buf, 11, LayoutCtx::truncated()), + Some(DisplayCoord::new(1, 5)) + ); + assert_eq!(view.pos_to_display(&buf, 12, LayoutCtx::truncated()), None); } #[test] fn ascii_display_to_pos_basic() { let (buf, view) = attached(b"hello\nworld"); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 0)), Some(0)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 5)), Some(5)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(1, 0)), Some(6)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(1, 5)), Some(11)); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 0), LayoutCtx::truncated()), + Some(0) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 5), LayoutCtx::truncated()), + Some(5) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(1, 0), LayoutCtx::truncated()), + Some(6) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(1, 5), LayoutCtx::truncated()), + Some(11) + ); // Past the visible end of a line: clamps. - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 10)), Some(5)); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 10), LayoutCtx::truncated()), + Some(5) + ); // Past the last line: None. - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(2, 0)), None); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(2, 0), LayoutCtx::truncated()), + None + ); } #[test] @@ -464,13 +786,25 @@ mod tests { let (buf, view) = attached("héllo".as_bytes()); assert_eq!(buf.len(), 6); // Position 0 -> col 0 - assert_eq!(view.pos_to_display(&buf, 0), Some(DisplayCoord::new(0, 0))); + assert_eq!( + view.pos_to_display(&buf, 0, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 0)) + ); // Position 1 (just after 'h') -> col 1 - assert_eq!(view.pos_to_display(&buf, 1), Some(DisplayCoord::new(0, 1))); + assert_eq!( + view.pos_to_display(&buf, 1, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 1)) + ); // Position 3 (just after 'é') -> col 2 - assert_eq!(view.pos_to_display(&buf, 3), Some(DisplayCoord::new(0, 2))); + assert_eq!( + view.pos_to_display(&buf, 3, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 2)) + ); // Position 6 (end) -> col 5 - assert_eq!(view.pos_to_display(&buf, 6), Some(DisplayCoord::new(0, 5))); + assert_eq!( + view.pos_to_display(&buf, 6, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 5)) + ); } #[test] @@ -478,20 +812,38 @@ mod tests { // "中文" each codepoint is 3 bytes UTF-8 and 2 columns wide. let (buf, view) = attached("中文".as_bytes()); assert_eq!(buf.len(), 6); - assert_eq!(view.pos_to_display(&buf, 0), Some(DisplayCoord::new(0, 0))); - assert_eq!(view.pos_to_display(&buf, 3), Some(DisplayCoord::new(0, 2))); - assert_eq!(view.pos_to_display(&buf, 6), Some(DisplayCoord::new(0, 4))); + assert_eq!( + view.pos_to_display(&buf, 0, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 0)) + ); + assert_eq!( + view.pos_to_display(&buf, 3, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 2)) + ); + assert_eq!( + view.pos_to_display(&buf, 6, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 4)) + ); } #[test] fn display_to_pos_jumps_over_wide_chars() { let (buf, view) = attached("中a".as_bytes()); // "中" is 2 cols wide, 3 bytes. "a" is 1 col, 1 byte. Total 4 bytes, 3 cols. - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 0)), Some(0)); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 0), LayoutCtx::truncated()), + Some(0) + ); // Asking for col 1 lands inside the wide char; we round to the next // codepoint boundary (col 2's start position). - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 2)), Some(3)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 3)), Some(4)); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 2), LayoutCtx::truncated()), + Some(3) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 3), LayoutCtx::truncated()), + Some(4) + ); } proptest! { @@ -502,8 +854,8 @@ mod tests { let buf = buf_with(content.as_bytes()); let view = TextView::new(&buf); let pos = (offset as u64).min(buf.len()); - if let Some(disp) = view.pos_to_display(&buf, pos) { - let back = view.display_to_pos(&buf, disp); + if let Some(disp) = view.pos_to_display(&buf, pos, LayoutCtx::truncated()) { + let back = view.display_to_pos(&buf, disp, LayoutCtx::truncated()); prop_assert_eq!(back, Some(pos)); } } @@ -515,30 +867,60 @@ mod tests { fn tab_at_start_advances_to_column_8() { let (buf, view) = attached(b"\tx"); // Position 0 (before tab) -> col 0 - assert_eq!(view.pos_to_display(&buf, 0), Some(DisplayCoord::new(0, 0))); + assert_eq!( + view.pos_to_display(&buf, 0, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 0)) + ); // Position 1 (after tab) -> col 8 - assert_eq!(view.pos_to_display(&buf, 1), Some(DisplayCoord::new(0, 8))); + assert_eq!( + view.pos_to_display(&buf, 1, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 8)) + ); // Position 2 (after 'x') -> col 9 - assert_eq!(view.pos_to_display(&buf, 2), Some(DisplayCoord::new(0, 9))); + assert_eq!( + view.pos_to_display(&buf, 2, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 9)) + ); } #[test] fn tab_in_middle_pads_to_next_stop() { // "ab\tcd": after 'b' col is 2, tab pads to col 8, 'c' at col 8. let (buf, view) = attached(b"ab\tcd"); - assert_eq!(view.pos_to_display(&buf, 0), Some(DisplayCoord::new(0, 0))); - assert_eq!(view.pos_to_display(&buf, 2), Some(DisplayCoord::new(0, 2))); - assert_eq!(view.pos_to_display(&buf, 3), Some(DisplayCoord::new(0, 8))); - assert_eq!(view.pos_to_display(&buf, 4), Some(DisplayCoord::new(0, 9))); - assert_eq!(view.pos_to_display(&buf, 5), Some(DisplayCoord::new(0, 10))); + assert_eq!( + view.pos_to_display(&buf, 0, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 0)) + ); + assert_eq!( + view.pos_to_display(&buf, 2, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 2)) + ); + assert_eq!( + view.pos_to_display(&buf, 3, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 8)) + ); + assert_eq!( + view.pos_to_display(&buf, 4, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 9)) + ); + assert_eq!( + view.pos_to_display(&buf, 5, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 10)) + ); } #[test] fn tab_aligned_input_advances_full_width() { // 8 chars then tab: the protocol tab stop advances col 8 to col 16. let (buf, view) = attached(b"01234567\tx"); - assert_eq!(view.pos_to_display(&buf, 8), Some(DisplayCoord::new(0, 8))); - assert_eq!(view.pos_to_display(&buf, 9), Some(DisplayCoord::new(0, 16))); + assert_eq!( + view.pos_to_display(&buf, 8, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 8)) + ); + assert_eq!( + view.pos_to_display(&buf, 9, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 16)) + ); } #[test] @@ -546,10 +928,391 @@ mod tests { // "\tx": col 0..8 are the tab; col 5 (inside the tab) should // round to byte 1 (the start of 'x'). let (buf, view) = attached(b"\tx"); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 0)), Some(0)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 5)), Some(1)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 8)), Some(1)); - assert_eq!(view.display_to_pos(&buf, DisplayCoord::new(0, 9)), Some(2)); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 0), LayoutCtx::truncated()), + Some(0) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 5), LayoutCtx::truncated()), + Some(1) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 8), LayoutCtx::truncated()), + Some(1) + ); + assert_eq!( + view.display_to_pos(&buf, DisplayCoord::new(0, 9), LayoutCtx::truncated()), + Some(2) + ); + } + + /// Under `wrap`, `pos_to_display` reports the visual row **within** + /// the source line, and `row` stays the source line. + #[test] + fn wrapped_coords_keep_row_as_the_source_line() { + let (buf, view) = attached(b"abcdefghij\nzz"); + let ctx = LayoutCtx { + cols: 4, + wrap: WrapMode::Wrap, + }; + // 'e' is byte 4: line 0, second visual row, column 0. + assert_eq!( + view.pos_to_display(&buf, 4, ctx), + Some(DisplayCoord::wrapped(0, 1, 0)) + ); + // The second SOURCE line is still row 1, not a visual row index. + assert_eq!( + view.pos_to_display(&buf, 11, ctx), + Some(DisplayCoord::wrapped(1, 0, 0)), + "row is the source line; a redefinition would have made this 3" + ); + } + + /// The wrap point belongs to column 0 of the next visual row, never + /// one past the end of the previous one (framing §7). + #[test] + fn the_wrap_point_is_owned_by_the_next_row() { + let (buf, view) = attached(b"abcdefgh"); + let ctx = LayoutCtx { + cols: 4, + wrap: WrapMode::Wrap, + }; + assert_eq!( + view.pos_to_display(&buf, 4, ctx), + Some(DisplayCoord::wrapped(0, 1, 0)), + "byte 4 is the start of row 1, not (row 0, col 4)" + ); + // The two DISTINCT adjacent codepoints across the break map + // distinctly: 'd' at (0,0,3) and 'e' at (0,1,0). + assert_eq!( + view.pos_to_display(&buf, 3, ctx), + Some(DisplayCoord::wrapped(0, 0, 3)) + ); + } + + /// Round trip is identity on every cursor boundary of a wrapped + /// line, and projection inside a multi-byte codepoint — the + /// contract framing §7 settled. + #[test] + fn wrapped_round_trip_is_identity_on_boundaries() { + let text = "abc\tde中fghijklmno"; + let (buf, view) = attached(text.as_bytes()); + for cols in [3_u32, 4, 5, 9] { + let ctx = LayoutCtx { + cols, + wrap: WrapMode::Wrap, + }; + for (byte, _) in text.char_indices() { + let coord = view + .pos_to_display(&buf, byte as u64, ctx) + .expect("in range"); + let back = view.display_to_pos(&buf, coord, ctx).expect("in range"); + assert_eq!( + back, byte as u64, + "cols={cols}: boundary {byte} did not round trip (via {coord:?})" + ); + } + } + } + + /// An interior byte projects to its codepoint's start, exactly as on + /// the unwrapped path — wrapping does not change that contract. + #[test] + fn wrapped_interior_bytes_project_to_the_codepoint_start() { + let text = "ab中cd"; + let (buf, view) = attached(text.as_bytes()); + let ctx = LayoutCtx { + cols: 4, + wrap: WrapMode::Wrap, + }; + // '中' starts at byte 2 and is three bytes long. + let at_start = view.pos_to_display(&buf, 2, ctx); + for interior in [3_u64, 4] { + assert_eq!( + view.pos_to_display(&buf, interior, ctx), + at_start, + "byte {interior} is inside the codepoint starting at 2" + ); + } + // ...and the projection is idempotent. + let coord = at_start.expect("in range"); + let back = view.display_to_pos(&buf, coord, ctx).expect("in range"); + assert_eq!(back, 2); + assert_eq!(view.pos_to_display(&buf, back, ctx), at_start); + } + + /// The identity control: with `truncated()` the mapping is exactly + /// what it was before wrapping existed. + #[test] + fn truncate_coords_are_unchanged() { + let (buf, view) = attached(b"abcdefghij"); + assert_eq!( + view.pos_to_display(&buf, 6, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 6)), + "no sub_row, and the column is the whole prefix width" + ); + } + + // ----------------------------------------------------------------- + // Line wrapping (QoL Stage 3, docs/long-lines-framing.md) + // ----------------------------------------------------------------- + + /// Render `text` into a `rows` x `cols` grid and return the glyph of + /// every cell, row-major. + fn render_grid(text: &[u8], rows: u32, cols: u32, wrap: WrapMode) -> Vec { + render_grid_from(text, rows, cols, wrap, 0) + } + + /// As [`render_grid`], but starting the viewport at byte `start` — + /// which under `Wrap` may sit partway down a wrapped line. + fn render_grid_from( + text: &[u8], + rows: u32, + cols: u32, + wrap: WrapMode, + start: u64, + ) -> Vec { + let (buf, mut view) = attached(text); + let n = (rows * cols) as usize; + let mut storage = vec![Cell::default(); n]; + let mut grid = CellGrid { + cells: &mut storage, + stride: cols, + size: CellSize::new(rows, cols), + }; + view.render( + &buf, + Viewport { + buffer_start: start, + buffer_end: buf.len(), + cell_origin: CellCoord::new(0, 0), + cell_size: CellSize::new(rows, cols), + gutter_w: 0, + folds: None, + wrap, + }, + &mut grid, + ); + storage.iter().map(|c| c.glyph.clone()).collect() + } + + fn row_text(glyphs: &[Glyph], cols: u32, row: u32) -> String { + glyphs + .iter() + .skip((row * cols) as usize) + .take(cols as usize) + .map(|g| match g { + Glyph::Char(c) => *c, + _ => ' ', + }) + .collect() + } + + /// `reached_buffer_end` is a local predicate, recorded by the walk. + /// + /// The scroll indicator needs `All`/`Top`/`Bot` without a row total + /// — which under wrapping does not exist — so it asks the walk + /// instead of counting. + #[test] + fn the_walk_reports_whether_it_reached_the_buffer_end() { + // Ten characters at four columns is three visual rows; a + // four-row viewport outruns the buffer. + let (buf, mut view) = attached(b"abcdefghij"); + let mut storage = vec![Cell::default(); 16]; + let mut grid = CellGrid { + cells: &mut storage, + stride: 4, + size: CellSize::new(4, 4), + }; + let vp = Viewport { + buffer_start: 0, + buffer_end: buf.len(), + cell_origin: CellCoord::new(0, 0), + cell_size: CellSize::new(4, 4), + gutter_w: 0, + folds: None, + wrap: WrapMode::Wrap, + }; + view.render(&buf, vp, &mut grid); + assert!( + view.reached_buffer_end(), + "three rows of content in four rows of viewport: the end is on screen" + ); + + // Two rows of viewport cannot hold three rows of content. + let mut small = vec![Cell::default(); 8]; + let mut small_grid = CellGrid { + cells: &mut small, + stride: 4, + size: CellSize::new(2, 4), + }; + view.render( + &buf, + Viewport { + cell_size: CellSize::new(2, 4), + ..vp + }, + &mut small_grid, + ); + assert!( + !view.reached_buffer_end(), + "the wrapped remainder is below the viewport" + ); + } + + /// A viewport too narrow to hold a wide glyph must not insert a + /// blank row before it. + /// + /// The wrap rule moves a double-width glyph to the next row when it + /// will not fit in the cells left. At one column it never fits + /// there either, so moving would leave row 0 empty and paint the + /// glyph clipped on row 1 — worse than clipping it in place. + #[test] + fn a_one_column_viewport_does_not_shove_wide_glyphs_down() { + let g = render_grid("中x".as_bytes(), 3, 1, WrapMode::Wrap); + assert_eq!( + g[0], + Glyph::Char('中'), + "the wide glyph belongs on row 0, clipped, not row 1" + ); + } + + /// A zero-width content area paints nothing and does not panic. + /// + /// Reachable when the line-number gutter consumes the whole window. + /// Under `Wrap` the walk's first `col >= max_cols` test is true + /// immediately, so without an explicit bail it advances a row and + /// then indexes column 0 of a zero-width grid. + #[test] + fn a_zero_width_viewport_paints_nothing() { + let (buf, mut view) = attached(b"abc\ndef"); + let mut storage: Vec = Vec::new(); + let mut grid = CellGrid { + cells: &mut storage, + stride: 0, + size: CellSize::new(4, 0), + }; + view.render( + &buf, + Viewport { + buffer_start: 0, + buffer_end: buf.len(), + cell_origin: CellCoord::new(0, 0), + cell_size: CellSize::new(4, 0), + gutter_w: 0, + folds: None, + wrap: WrapMode::Wrap, + }, + &mut grid, + ); + assert!(storage.is_empty(), "nothing to paint, and nothing painted"); + } + + /// The reported defect: a line wider than the window is readable. + #[test] + fn a_long_line_continues_on_the_following_rows() { + let g = render_grid(b"abcdefghij", 4, 4, WrapMode::Wrap); + assert_eq!(row_text(&g, 4, 0), "abcd"); + assert_eq!(row_text(&g, 4, 1), "efgh"); + assert_eq!(row_text(&g, 4, 2), "ij "); + assert_eq!( + row_text(&g, 4, 3), + " ", + "no fifth row of a ten-char line" + ); + } + + /// The identity control: the same input under `Truncate` is exactly + /// what the pre-wrap renderer produced. + #[test] + fn truncate_still_clips_at_the_edge() { + let g = render_grid(b"abcdefghij", 4, 4, WrapMode::Truncate); + assert_eq!(row_text(&g, 4, 0), "abcd"); + assert_eq!( + row_text(&g, 4, 1), + " ", + "one row per line, remainder clipped" + ); + } + + /// Wrapping is per source line: a second line starts a new row + /// rather than continuing the first. + #[test] + fn each_source_line_starts_its_own_row() { + let g = render_grid( + b"abcde +xy", + 4, + 4, + WrapMode::Wrap, + ); + assert_eq!(row_text(&g, 4, 0), "abcd"); + assert_eq!(row_text(&g, 4, 1), "e "); + assert_eq!(row_text(&g, 4, 2), "xy "); + } + + /// A double-width glyph with one cell left moves to the next row + /// whole, rather than being split or half-painted. + #[test] + fn a_wide_char_that_does_not_fit_moves_down_whole() { + // 3 columns: "ab" fills 0..2, leaving one cell — too narrow for + // the 2-column CJK glyph. + let g = render_grid("ab中".as_bytes(), 3, 3, WrapMode::Wrap); + assert_eq!(row_text(&g, 3, 0), "ab ", "the odd cell stays blank"); + assert_eq!( + g[3], + Glyph::Char('中'), + "the glyph starts the next row instead of straddling" + ); + assert_eq!(g[4], Glyph::Continuation, "and keeps its continuation cell"); + } + + /// A tab fills to the row's end and stops; it never spans a wrap. + /// Column 0 of the next row is itself a tab stop, so alignment + /// survives the break. + #[test] + fn a_tab_fills_to_the_row_end_and_stops() { + let g = render_grid(b"ab z", 4, 4, WrapMode::Wrap); + assert_eq!(row_text(&g, 4, 0), "ab ", "tab pads to the edge"); + assert_eq!( + row_text(&g, 4, 1), + "z ", + "and the next char resumes at col 0" + ); + } + + /// The byte anchor may sit partway down a wrapped line, which is + /// why `view_top`'s sub-line component is a byte (framing Q#LL6). + #[test] + fn the_viewport_can_start_partway_down_a_wrapped_line() { + // Byte 4 is 'e', the first character of the second visual row. + let g = render_grid_from(b"abcdefghij", 2, 4, WrapMode::Wrap, 4); + assert_eq!(row_text(&g, 4, 0), "efgh", "the first row is skipped"); + assert_eq!(row_text(&g, 4, 1), "ij "); + } + + /// `row_of_byte` and the painter must agree, or the viewport scrolls + /// to a row the renderer does not draw. They share `advance_wrapped` + /// precisely so this cannot drift; the witness pins it anyway. + #[test] + fn the_anchor_row_matches_where_the_text_is_painted() { + let text = "ab cd中efghij"; + let (buf, view) = attached(text.as_bytes()); + for cols in [3_u32, 4, 5, 8] { + for (byte, _) in text.char_indices() { + let row = view.row_of_byte(&buf, 0, byte as u64, cols); + // Anchoring the viewport at that byte must put the + // character on the viewport's FIRST row. + let g = render_grid_from(text.as_bytes(), 3, cols, WrapMode::Wrap, byte as u64); + let full = render_grid(text.as_bytes(), 12, cols, WrapMode::Wrap); + let expect = row_text(&full, cols, row); + assert_eq!( + row_text(&g, cols, 0), + expect, + "cols={cols} byte={byte}: anchor row {row} is not the row painted first" + ); + } + } } #[test] @@ -570,6 +1333,7 @@ mod tests { cell_size: CellSize::new(1, 16), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -599,6 +1363,7 @@ mod tests { cell_size: CellSize::new(1, 16), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -631,6 +1396,7 @@ mod tests { cell_size: CellSize::new(5, 5), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); @@ -664,6 +1430,7 @@ mod tests { cell_size: CellSize::new(1, 5), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }, &mut grid, ); diff --git a/src/view.rs b/src/view.rs index e3e4b6c..ab77cf1 100644 --- a/src/view.rs +++ b/src/view.rs @@ -108,18 +108,123 @@ impl InterceptContext { /// and inline expansions appear. #[derive(Copy, Clone, Eq, PartialEq, Debug, Default)] pub struct DisplayCoord { - /// 0-based row. + /// 0-based **source line** index. + /// + /// Deliberately still the source line under wrapping, not a visual + /// row. Redefining it would have broken every existing consumer — + /// `overlay_paint`'s `row - view_top`, vertical motion's bounds + /// check — with no compile error. Adding [`Self::sub_row`] beside it + /// instead leaves those consumers *correct*, not merely findable. pub row: u32, - /// 0-based column. + /// Which visual row **within** `row`, when the line wraps. + /// + /// `0` for every unwrapped line and under + /// [`WrapMode::Truncate`](crate::view::WrapMode::Truncate), which is + /// what makes this field additive: code that has never heard of + /// wrapping keeps computing the right answer. + pub sub_row: u32, + /// 0-based column, within the visual row named by `sub_row`. pub col: u32, } impl DisplayCoord { - /// Construct a display coordinate. + /// Construct a display coordinate on a line's first visual row. #[must_use] pub const fn new(row: u32, col: u32) -> Self { - Self { row, col } + Self { + row, + sub_row: 0, + col, + } } + + /// Construct a display coordinate on a specific visual row of a + /// wrapped line. + #[must_use] + pub const fn wrapped(row: u32, sub_row: u32, col: u32) -> Self { + Self { row, sub_row, col } + } +} + +/// The layout facts a coordinate mapping needs, which the mapping +/// itself cannot know. +/// +/// # Why this is a required parameter +/// +/// `pos_to_display` took `(&self, buf, pos)` and had no notion of the +/// grid at all — so under wrapping it could not compute a visual row, +/// and `display_to_pos` could not invert one. Passing the missing +/// facts as a required argument is deliberate: it makes the compiler +/// enumerate every call site rather than leaving an audit to grep. +/// +/// That is the opposite choice from [`DisplayCoord::sub_row`], and for +/// the opposite reason. Enforcement is possible on the way in, so it is +/// taken; it is not possible on the way out, so the output is made +/// correct-by-default instead. +#[derive(Copy, Clone, Eq, PartialEq, Debug, Default)] +pub struct LayoutCtx { + /// Width in cells of the content area the text wraps within. + /// + /// `0` means "not rendered yet" — the same convention + /// `Window::last_visible_rows` uses — and is treated as unwrapped, + /// since a viewport with no columns has no rows to distinguish. + pub cols: u32, + /// The window's resolved wrap mode. + pub wrap: WrapMode, +} + +impl LayoutCtx { + /// The identity context: no wrapping, width irrelevant. + /// + /// Every pre-wrap caller means this, and saying so explicitly is + /// what makes those call sites readable as decisions rather than + /// oversights. + #[must_use] + pub const fn truncated() -> Self { + Self { + cols: 0, + wrap: WrapMode::Truncate, + } + } + + /// Whether this context actually wraps. + #[must_use] + pub const fn wrapping(self) -> bool { + matches!(self.wrap, WrapMode::Wrap) && self.cols > 0 + } +} + +/// How a line wider than the viewport is shown --- the long-lines +/// stage, `docs/long-lines-framing.md`. +/// +/// # Why the renderer is told, rather than asking +/// +/// This is the *resolved* mode, not the setting's name. `ui.line-wrap` +/// is **buffer-local** (framing Q#LL2), and the config registry has no +/// ambient current buffer by design (`config_registry.rs`, Q#CR4) — a +/// caller wanting buffer-aware behavior must pass the `BufferId`. The +/// render driver holds both the registry and the buffer, resolves once +/// per window per frame, and puts the answer here. Views stay +/// config-agnostic, exactly as they do for folds. +/// +/// # `Truncate` is the identity case, deliberately +/// +/// Every behavior predating this type is `Truncate`, and it must stay +/// byte-identical under it — which is what lets the wrap work be +/// verified against the existing suite rather than against new +/// assertions. +#[derive(Copy, Clone, Eq, PartialEq, Debug, Default, Hash)] +pub enum WrapMode { + /// One source line per row; the remainder is clipped at the right + /// edge. What every pre-Stage-3 caller did. + #[default] + Truncate, + /// A line longer than the viewport continues on the following rows. + /// + /// Character wrap, not word wrap (framing Q#LL5): it matches + /// Emacs's default, and it is the only break rule both frontends + /// can implement identically without pulling UAX #14 into the grid. + Wrap, } /// What to render and where. @@ -152,6 +257,14 @@ pub struct Viewport<'a> { /// split may show different buffers) and hands the same shared /// reference to every painter of that window. pub folds: Option<&'a VisibleLineMap>, + /// How lines wider than `cell_size.cols` are shown, already + /// resolved for this window's buffer (see [`WrapMode`]). + /// + /// A required field rather than a defaulted one on purpose: every + /// construction site has to state which behavior it means, so the + /// pre-Stage-3 sites read as *deliberately* unwrapped rather than + /// merely untouched. + pub wrap: WrapMode, } impl Viewport<'_> { @@ -268,14 +381,24 @@ pub trait View { /// view holds a meaningful mapping for that position. /// /// Default: returns `None` (view has no opinion). - fn pos_to_display(&self, _buf: &Buffer, _pos: Position) -> Option { + fn pos_to_display( + &self, + _buf: &Buffer, + _pos: Position, + _ctx: LayoutCtx, + ) -> Option { None } /// Translate a display coordinate back to a buffer byte position. /// /// Default: returns `None`. - fn display_to_pos(&self, _buf: &Buffer, _coord: DisplayCoord) -> Option { + fn display_to_pos( + &self, + _buf: &Buffer, + _coord: DisplayCoord, + _ctx: LayoutCtx, + ) -> Option { None } @@ -360,6 +483,7 @@ mod tests { cell_size: CellSize::new(10, 10), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; assert_eq!(vp.row_offset_of(4, 4), Some(0)); assert_eq!(vp.row_offset_of(4, 9), Some(5)); @@ -380,6 +504,7 @@ mod tests { cell_size: CellSize::new(10, 10), gutter_w: 0, folds: Some(&map), + wrap: WrapMode::Truncate, }; assert_eq!(vp.row_offset_of(0, 1), Some(1), "the head keeps its row"); assert_eq!(vp.row_offset_of(0, 3), None, "hidden lines have no row"); diff --git a/src/window.rs b/src/window.rs index 2ac0087..4b6514d 100644 --- a/src/window.rs +++ b/src/window.rs @@ -378,6 +378,33 @@ pub struct Window { /// render. Updated by the renderer; consumed by `cursor.page-down` /// / `cursor.page-up`. `0` until the first render lands. pub last_visible_rows: u32, + /// Width in cells of this window's **content** area at last render + /// — the text columns, with the line-number gutter already + /// subtracted. Updated by the renderer alongside + /// [`Self::last_visible_rows`]; `0` until the first render lands. + /// + /// Content, not window, width: the gutter grows at the line-count + /// digit boundary (9 -> 10, 99 -> 100), so the two differ and only + /// this one is where text actually wraps. + /// + /// Needed because line wrapping makes the cursor's display + /// coordinate width-dependent, and the callers that ask for it — + /// vertical motion, paging, overlay placement — hold a window but + /// not the frame's geometry. `last_visible_rows` established this + /// pattern for rows; wrapping needs the other axis. + pub last_content_cols: u32, + /// Wrap mode this window's buffer resolved to at last render. + /// + /// Recorded by the driver beside [`Self::last_content_cols`], for + /// the same reason: the mode is **buffer-local** config and the + /// registry has no ambient buffer, so only the driver can resolve + /// it — but vertical motion, paging and overlay placement all need + /// it and hold a window rather than a registry. + /// + /// One resolution, recorded once, consumed everywhere. The + /// alternative — each consumer resolving for itself — is how two + /// callers end up disagreeing about the same buffer. + pub last_wrap: crate::view::WrapMode, /// Line-number gutter mode for this window (UX gutter arc). `Off` by /// default → no gutter, no coordinate change. pub line_numbers: LineNumberMode, @@ -401,6 +428,8 @@ impl Window { view_top: 0, goal_col: None, last_visible_rows: 0, + last_content_cols: 0, + last_wrap: crate::view::WrapMode::Truncate, line_numbers: LineNumberMode::Off, params: WindowParams::default(), } @@ -412,6 +441,18 @@ impl Window { self.params.is_side() } + /// The layout facts a coordinate mapping needs for this window. + /// + /// Reads what the last render recorded, so every consumer sees the + /// same answer the renderer used rather than deriving its own. + #[must_use] + pub fn layout_ctx(&self) -> crate::view::LayoutCtx { + crate::view::LayoutCtx { + cols: self.last_content_cols, + wrap: self.last_wrap, + } + } + /// Width in cells this window's line-number gutter occupies, or `0` /// when disabled (UX gutter arc, Q#UX3). `digits(line_count) + PAD`; /// the renderer caps this against the window width and applies it as a diff --git a/tests/bottom_panel_stage2b_daemon_acceptance.rs b/tests/bottom_panel_stage2b_daemon_acceptance.rs index cc80422..e73109a 100644 --- a/tests/bottom_panel_stage2b_daemon_acceptance.rs +++ b/tests/bottom_panel_stage2b_daemon_acceptance.rs @@ -32,7 +32,7 @@ use pmacs::editor_core::GeometryUpdate; use pmacs::protocol::{FrontendId, InstanceMessage, PROTOCOL_VERSION}; use pmacs::semantic_render::SemanticRenderState; use pmacs::window::{FrontendView, Layout, Window, WindowId}; -use pmacs_protocol::panel::{PanelFrame, PanelFramePayload}; +use pmacs_protocol::panel::{PANEL_MIN_VERSION, PanelFrame, PanelFramePayload}; // --------------------------------------------------------------------------- // Harness @@ -740,10 +740,25 @@ fn acc51_a_pre_panel_semantic_frontend_is_never_sent_a_panel_frame() { ); } +/// A peer below [`PANEL_MIN_VERSION`] gets no `PanelFrame` even when the +/// daemon could build one for it. +/// +/// **Anchored on `PANEL_MIN_VERSION - 1`, not `PROTOCOL_VERSION - 1`.** +/// The original spelling was the latter, which expressed an *absolute* +/// contract — "older than the version that introduced panel frames" — +/// as arithmetic on a *moving* constant. It held only while +/// `PROTOCOL_VERSION` happened to equal `PANEL_MIN_VERSION`, and the +/// long-lines lane's bump to v22 made `PROTOCOL_VERSION - 1` equal +/// `PANEL_MIN_VERSION` exactly: the fixture's "old" peer became +/// panel-capable, so the daemon correctly sent a frame and the test +/// correctly failed. The production code was never wrong. +/// +/// `src/daemon.rs` and `pmacs-gpu/src/main.rs` already spell this +/// `PANEL_MIN_VERSION - 1` in five places; this was the one outlier. #[test] -fn acc51_a_v20_peer_is_sent_no_panel_frame_even_when_capable() { +fn acc51_a_sub_panel_version_peer_is_sent_no_panel_frame_even_when_capable() { let mut session = Session::new(); - session.render = SemanticRenderState::for_peer(FID, PROTOCOL_VERSION - 1); + session.render = SemanticRenderState::for_peer(FID, PANEL_MIN_VERSION - 1); session.declare(1, ROWS, COLS); open_panel(&session, "*panel*", 4); diff --git a/tests/bottom_panel_stage2b_gpu_acceptance.rs b/tests/bottom_panel_stage2b_gpu_acceptance.rs index e11493b..257680a 100644 --- a/tests/bottom_panel_stage2b_gpu_acceptance.rs +++ b/tests/bottom_panel_stage2b_gpu_acceptance.rs @@ -336,13 +336,31 @@ fn one_daemon_serves_a_v21_panel_session_and_a_shipped_v20_client() { /// and the counter-offer is what reaches the current wire. #[test] fn the_baseline_stays_and_the_counter_offer_activates() { - assert_eq!(PROTOCOL_VERSION, 21); + // A deliberate tripwire: bumping the wire must be a conscious edit + // here, not a silent one. v22 is `LineWrapFacts` (long-lines Stage 3). + assert_eq!(PROTOCOL_VERSION, 22); assert_eq!( ADVERTISED_PROTOCOL_VERSION, 20, "moving this is the incompatible act the mechanism exists to avoid" ); const { assert!(PROTOCOL_VERSION > ADVERTISED_PROTOCOL_VERSION) }; - assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION); + + // Panel frames are gated ABOVE the advertised floor and at or below + // this binary's wire. Both halves are durable properties of the + // gate. + // + // This replaces `assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION)`, + // which asserted a **coincidence**: panel frames were the newest + // feature when it was written, so their minimum happened to equal + // the current wire. Any later feature falsifies that — v22 is the + // first, and the equality would have had to be edited on every + // subsequent bump while telling a reader something that was never + // the contract. + // `const` blocks, matching the line above: these are compile-time + // constants, so a runtime `assert!` is both a clippy error and a + // weaker check than the language already offers. + const { assert!(PANEL_MIN_VERSION > ADVERTISED_PROTOCOL_VERSION) }; + const { assert!(PANEL_MIN_VERSION <= PROTOCOL_VERSION) }; // The current baseline is answered with this binary's own version. assert_eq!( @@ -584,8 +602,24 @@ fn a54_real_daemon_real_pty_and_headless_gpu_render_one_panel_hosted_terminal() "a deadline-driven pass must not read as success: {text}" ); // The activation, end to end on a real socket. - assert_eq!(fact("session_protocol_version"), "21", "{text}"); + // + // The session version is compared against `PROTOCOL_VERSION` rather + // than the literal "21" it used to pin: what the counter-offer + // activates is *this binary's* wire, so the literal was only ever + // correct while the panel stage was the newest one. The baseline + // stays a literal, because 20 not moving IS the claim. + assert_eq!( + fact("session_protocol_version"), + PROTOCOL_VERSION.to_string(), + "{text}" + ); assert_eq!(fact("baseline_protocol_version"), "20", "{text}"); + // …and that negotiated version is panel-capable, which is the part + // "21" used to carry implicitly. + assert!( + number("session_protocol_version") >= PANEL_MIN_VERSION, + "the negotiated wire must reach the panel minimum: {text}" + ); // The band is real: declared, projected, focused, and carrying the child. assert!( diff --git a/tests/bottom_panel_stage2b_protocol_acceptance.rs b/tests/bottom_panel_stage2b_protocol_acceptance.rs index 55bbc12..2117de0 100644 --- a/tests/bottom_panel_stage2b_protocol_acceptance.rs +++ b/tests/bottom_panel_stage2b_protocol_acceptance.rs @@ -14,7 +14,7 @@ use pmacs_protocol::message::{ AttachRequest, FrontendEvent, Hello, InstanceMessage, Modifiers, MouseButton, MouseKind, }; use pmacs_protocol::panel::{ - MAX_PANEL_VISIBLE_CELLS, PanelFrame, PanelFrameError, PanelFramePayload, + MAX_PANEL_VISIBLE_CELLS, PANEL_MIN_VERSION, PanelFrame, PanelFrameError, PanelFramePayload, }; use pmacs_protocol::terminal::{ MAX_TERMINAL_COLS, TerminalFrame, TerminalFrameError, TerminalProcessState, @@ -107,8 +107,18 @@ fn terminal_frame(rows: u32, cols: u32) -> TerminalFrame { #[test] fn the_panel_stage_takes_protocol_v21() { - assert_eq!(PROTOCOL_VERSION, 21); + // The panel stage's own version, which does not move when a later + // feature appends to the wire. + // + // This was `assert_eq!(PROTOCOL_VERSION, 21)` — the CURRENT wire + // used as a proxy for the panel stage's version. The two were equal + // only until the next feature landed (v22, `LineWrapFacts`), and the + // proxy then failed in a test whose own name says what it means to + // pin. `PANEL_MIN_VERSION` is that constant. + assert_eq!(PANEL_MIN_VERSION, 21); assert!(SUPPORTED_PROTOCOL_VERSIONS.contains(&21)); + // This binary must be able to speak the stage it implements. + const { assert!(PROTOCOL_VERSION >= PANEL_MIN_VERSION) }; // The advertised version is a compatibility BASELINE, and Stage 2B-3 // made that permanent rather than temporary: the server-first Hello // reaches an already-shipped frontend before that frontend can send diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index 1b38abe..1b5150c 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -270,7 +270,7 @@ fn render_active_window_to_grid( cols: u32, ) -> Vec { use pmacs::cell::{Cell, CellGrid, CellSize}; - use pmacs::view::{View, Viewport}; + use pmacs::view::{View, Viewport, WrapMode}; use pmacs::window::Rect; let mut core = state.core.borrow_mut(); @@ -289,6 +289,7 @@ fn render_active_window_to_grid( cell_size: CellSize::new(rect.size.rows, rect.size.cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let mut grid = CellGrid { cells: &mut backing, diff --git a/tests/line_wrap_acceptance.rs b/tests/line_wrap_acceptance.rs new file mode 100644 index 0000000..4036295 --- /dev/null +++ b/tests/line_wrap_acceptance.rs @@ -0,0 +1,223 @@ +//! Line-wrap acceptance (long lines, `docs/long-lines-framing.md`). +//! +//! `ui.line-wrap` is **buffer-local** (Q#LL2), and buffer-local is the +//! whole point rather than a nicety: prose wants wrapping and a log file +//! usually does not, in the same session. +//! +//! Which makes the toggle command's failure mode specific and invisible +//! in any single-buffer test. `pmacs.config.get(name)` reads the global +//! chain and `pmacs.config.set(name, ...)` writes the global layer, so a +//! toggle built from that pair reports and flips the **global** value. +//! In a buffer pinned to `truncate` it would leave that buffer exactly +//! as it was while silently changing every buffer that had not been +//! pinned — the opposite of what the user asked for, in both directions +//! at once. +//! +//! So these tests use a second buffer, and a buffer pinned against +//! the global layer, to make both halves of that failure visible. + +use std::path::Path; + +use pmacs::bootstrap::BootstrapRoots; +use pmacs::editor::EditorState; + +fn session(name: &str) -> EditorState { + let base = Path::new(env!("CARGO_TARGET_TMPDIR")) + .join("line-wrap") + .join(name); + let _ = std::fs::remove_dir_all(&base); + let roots = BootstrapRoots::isolated_under(&base); + for (_, dir) in roots.child_env() { + std::fs::create_dir_all(&dir).expect("create controlled root"); + } + let state = EditorState::new_with_roots(&roots); + state.install_state_dirs(); + state +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +/// The resolved mode for the buffer currently in the window. +fn mode_here(s: &EditorState) -> String { + eval( + s, + "return pmacs.config.get('ui.line-wrap', pmacs.window.buffer())", + ) +} + +/// The global layer's value, which is *not* what a buffer necessarily +/// resolves to. +fn mode_global(s: &EditorState) -> String { + eval(s, "return pmacs.config.get('ui.line-wrap')") +} + +/// Put `text` in the window's current buffer by typing it, which is the +/// only path Lua exposes and also the one a user takes. +fn fill_active(s: &EditorState, text: &str) { + for ch in text.chars() { + exec( + s, + &format!("pmacs.editor.insert_char_over_region({})", ch as u32), + ); + } +} + +/// Render one frame and return the text of the first `rows` grid rows, +/// reconstructed from the emitted `CellDelta` spans. +/// +/// Two reasons for going through `RenderState` and the wire rather than +/// calling `TextView::render` with a hand-built viewport. The defect +/// this guards against lived in the *driver*, between the resolved mode +/// and the viewport it built — a test that constructed its own viewport +/// would have passed against it. And the spans are what the TUI +/// actually consumes, so this asserts on the bytes that reach a screen. +fn render_rows(s: &EditorState, rows: u32, cols: u32) -> Vec { + use std::collections::HashMap; + + let size = pmacs::cell::CellSize::new(rows, cols); + let mut rs = pmacs::instance_render::RenderState::new(size); + let msgs = rs.render_frame(s, pmacs::protocol::FrontendId::LOCAL, &HashMap::new(), &[]); + + let mut grid = vec![vec![' '; cols as usize]; rows as usize]; + for msg in &msgs { + if let pmacs_protocol::InstanceMessage::CellDelta { spans, .. } = msg { + for span in spans { + for (i, cell) in span.cells.iter().enumerate() { + let r = span.start.row as usize; + let c = span.start.col as usize + i; + if r < rows as usize + && c < cols as usize + && let pmacs::cell::Glyph::Char(ch) = cell.glyph + { + grid[r][c] = ch; + } + } + } + } + } + grid.into_iter().map(|r| r.into_iter().collect()).collect() +} + +#[test] +fn the_default_is_wrap() { + let s = session("default"); + assert_eq!(mode_global(&s), "wrap"); + assert_eq!( + mode_here(&s), + "wrap", + "with no buffer-local override, a buffer resolves to the global default" + ); +} + +#[test] +fn only_wrap_and_truncate_are_accepted() { + let s = session("enum"); + let err: bool = eval( + &s, + "local ok = pcall(pmacs.config.set, 'ui.line-wrap', 'sideways'); return not ok", + ); + assert!( + err, + "a closed choice set makes an unknown mode impossible rather than handled" + ); +} + +/// The finding this suite exists for: the toggle must move the buffer +/// it is invoked in, and **only** that buffer. +/// +/// There is no Lua buffer-switch, so "the other buffer" is a second +/// buffer that exists but is not shown — which is the case that matters +/// anyway: a global write reaches every buffer without an override of +/// its own, shown or not. +#[test] +fn the_toggle_moves_this_buffer_and_leaves_the_other_alone() { + let s = session("two_buffers"); + exec(&s, "OTHER = pmacs.buffer.create('other')"); + assert_eq!( + eval::(&s, "return pmacs.config.get('ui.line-wrap', OTHER)"), + "wrap", + "precondition: the second buffer starts at the default" + ); + + exec(&s, "pmacs.command.invoke('ui.toggle-line-wrap')"); + assert_eq!(mode_here(&s), "truncate", "the invoking buffer moved"); + + assert_eq!( + eval::(&s, "return pmacs.config.get('ui.line-wrap', OTHER)"), + "wrap", + "toggling in one buffer must not change another" + ); + assert_eq!( + mode_global(&s), + "wrap", + "a buffer-local toggle must not write the global layer — doing so \ + would change every buffer with no override of its own" + ); +} + +/// A buffer pinned to `truncate` must toggle back to `wrap`, reading its +/// own value rather than the global one. +/// +/// This is the half a global-read toggle gets wrong even if its write +/// were harmless: it would see `wrap` globally, decide the next mode is +/// `truncate`, and leave the pinned buffer exactly as it was. +#[test] +fn a_pinned_buffer_toggles_from_its_own_value() { + let s = session("pinned"); + exec( + &s, + "pmacs.config.set_local(pmacs.window.buffer(), 'ui.line-wrap', 'truncate')", + ); + assert_eq!(mode_here(&s), "truncate"); + assert_eq!(mode_global(&s), "wrap", "precondition: the layers differ"); + + exec(&s, "pmacs.command.invoke('ui.toggle-line-wrap')"); + assert_eq!( + mode_here(&s), + "wrap", + "the toggle read this buffer's value, not the global one" + ); +} + +/// The default must reach the **rendered cells**, not merely the +/// resolved value. +/// +/// This is the gap review found: the frame resolved `ui.line-wrap`, +/// recorded it on the window, and fed it to the coordinate mapping and +/// the scroll indicator — while the viewport handed the renderer a +/// hard-coded `Truncate`. Every "is the mode right?" assertion passed +/// and the text was still clipped. So this test reads the grid. +#[test] +fn the_default_actually_wraps_the_painted_text() { + let s = session("rendered_default"); + // Wider than the viewport below, and distinctive. + fill_active(&s, "ABCDEFGHIJKLMNOP"); + let rows = render_rows(&s, 4, 4); + assert_eq!(rows[0], "ABCD"); + assert_eq!( + rows[1], "EFGH", + "the default is `wrap`, so the remainder continues on the next row \ + — a hard-coded Truncate in the viewport leaves this blank" + ); +} + +/// And `truncate` still clips, so the witness above is discriminating +/// rather than merely true. +#[test] +fn truncate_clips_the_painted_text() { + let s = session("rendered_truncate"); + fill_active(&s, "ABCDEFGHIJKLMNOP"); + exec( + &s, + "pmacs.config.set_local(pmacs.window.buffer(), 'ui.line-wrap', 'truncate')", + ); + let rows = render_rows(&s, 4, 4); + assert_eq!(rows[0], "ABCD"); + assert_eq!(rows[1], " ", "truncate keeps one row per source line"); +} diff --git a/tests/listview_acceptance.rs b/tests/listview_acceptance.rs index e845fce..6653fbd 100644 --- a/tests/listview_acceptance.rs +++ b/tests/listview_acceptance.rs @@ -144,7 +144,7 @@ fn set_read_only(s: &EditorState, id: BufferId, value: bool) { /// bites: the rope is right and the screen is not. fn paint_active_window(s: &EditorState, rows: u32, cols: u32) -> Vec { use pmacs::cell::{Cell, CellGrid, CellSize}; - use pmacs::view::{View, Viewport}; + use pmacs::view::{View, Viewport, WrapMode}; use pmacs::window::Rect; let mut core = s.core.borrow_mut(); @@ -162,6 +162,7 @@ fn paint_active_window(s: &EditorState, rows: u32, cols: u32) -> Vec "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); +} diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index bf6b7f2..492de49 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -483,7 +483,7 @@ fn render_active_window_to_grid( cols: u32, ) -> Vec { use pmacs::cell::{Cell, CellGrid, CellSize}; - use pmacs::view::{View, Viewport}; + use pmacs::view::{View, Viewport, WrapMode}; use pmacs::window::Rect; let mut core = state.core.borrow_mut(); @@ -502,6 +502,7 @@ fn render_active_window_to_grid( cell_size: CellSize::new(rect.size.rows, rect.size.cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let mut grid = CellGrid { cells: &mut backing, diff --git a/tests/statusline_segments_acceptance.rs b/tests/statusline_segments_acceptance.rs index 2df0161..b583c83 100644 --- a/tests/statusline_segments_acceptance.rs +++ b/tests/statusline_segments_acceptance.rs @@ -788,15 +788,21 @@ fn a12_builtin_lsp_provider_tracks_real_attachment_and_unknown_label() { fn a13_17_26_protocol_semantic_init_late_join_and_version_cost() { // Vterm Stage 3 appended the terminal family as v19; GPU initial targets // appended the semantic bootstrap family as v20; bottom-panel Stage 2B-1 - // appended the panel family as v21. This acceptance owns the STATUSLINE + // appended the panel family as v21; long-lines Stage 3 appended + // `LineWrapFacts` as v22. This acceptance owns the STATUSLINE // variant's placement and gate, so it tracks the current wire version // rather than pinning 18: the v18 floor it actually cares about is asserted // below and in `peer_accepts_statusline_message`. - assert_eq!(PROTOCOL_VERSION, 21); - for version in 6..=21 { + // + // Tracking the current wire is deliberate, so every bump edits these + // three lines on purpose. The ceiling assertion is the load-bearing + // one — it says the supported set ENDS here, which is what makes an + // accidentally-widened set a failure rather than a silent pass. + assert_eq!(PROTOCOL_VERSION, 22); + for version in 6..=22 { assert!(is_supported_protocol_version(version)); } - assert!(!is_supported_protocol_version(22)); + assert!(!is_supported_protocol_version(23)); let sample = InstanceMessage::StatuslineSegments { buffer_id: BufferId::from_raw(9), left: vec![StatuslineSegment { diff --git a/tests/tab_width_acceptance.rs b/tests/tab_width_acceptance.rs index e153983..90e84db 100644 --- a/tests/tab_width_acceptance.rs +++ b/tests/tab_width_acceptance.rs @@ -6,7 +6,7 @@ use pmacs::buffer::{Buffer, BufferId}; use pmacs::cell::{Cell, CellCoord, CellGrid, CellSize, Glyph, Style}; use pmacs::overlay::{BufferStyleOverlay, BufferStyleSpan, SharedBufferStyleSpans}; use pmacs::text_view::TextView; -use pmacs::view::{DisplayCoord, View, Viewport}; +use pmacs::view::{DisplayCoord, LayoutCtx, View, Viewport, WrapMode}; fn viewport(rows: u32, cols: u32, buffer_end: u64) -> Viewport<'static> { Viewport { @@ -16,6 +16,7 @@ fn viewport(rows: u32, cols: u32, buffer_end: u64) -> Viewport<'static> { cell_size: CellSize::new(rows, cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, } } @@ -36,10 +37,16 @@ fn plain_text_projects_tabs_without_changing_source_bytes() { let buf = Buffer::from_bytes(BufferId::next(), "tabs", source); let cells = render_text(&buf, 3, 20); let view = TextView::new(&buf); - assert_eq!(view.pos_to_display(&buf, 1), Some(DisplayCoord::new(0, 8))); - assert_eq!(view.pos_to_display(&buf, 11), Some(DisplayCoord::new(1, 8))); assert_eq!( - view.pos_to_display(&buf, 22), + view.pos_to_display(&buf, 1, LayoutCtx::truncated()), + Some(DisplayCoord::new(0, 8)) + ); + assert_eq!( + view.pos_to_display(&buf, 11, LayoutCtx::truncated()), + Some(DisplayCoord::new(1, 8)) + ); + assert_eq!( + view.pos_to_display(&buf, 22, LayoutCtx::truncated()), Some(DisplayCoord::new(2, 16)) ); diff --git a/tests/terminal_copy_mode_acceptance.rs b/tests/terminal_copy_mode_acceptance.rs index ecb6e69..13f3758 100644 --- a/tests/terminal_copy_mode_acceptance.rs +++ b/tests/terminal_copy_mode_acceptance.rs @@ -468,7 +468,7 @@ fn render_active_window_to_grid( cols: u32, ) -> Vec { use pmacs::cell::{Cell, CellGrid}; - use pmacs::view::{View, Viewport}; + use pmacs::view::{View, Viewport, WrapMode}; use pmacs::window::Rect; let mut core = state.core.borrow_mut(); @@ -486,6 +486,7 @@ fn render_active_window_to_grid( cell_size: CellSize::new(rows, cols), gutter_w: 0, folds: None, + wrap: WrapMode::Truncate, }; let mut grid = CellGrid { cells: &mut backing, diff --git a/tests/vterm_stage3_acceptance.rs b/tests/vterm_stage3_acceptance.rs index 0305146..b5a71c5 100644 --- a/tests/vterm_stage3_acceptance.rs +++ b/tests/vterm_stage3_acceptance.rs @@ -737,10 +737,18 @@ fn a37_real_daemon_real_pty_and_headless_gpu_render_one_terminal_session() { // which is exactly the incompatible change this mechanism exists to // avoid — and asserting only the baseline would pass with the whole // activation missing. + // Compared against `PROTOCOL_VERSION`, not the literal "21": the + // counter-offer activates THIS BINARY's wire, so the literal held + // only while the panel stage was the newest one (long-lines Stage 3 + // appended v22). The baseline assertion below stays literal, + // because 20 not moving is the actual claim. + // Fully qualified: this file imports `PROTOCOL_VERSION` inside a + // different test's scope, not at module level. + let session_version = pmacs_protocol::PROTOCOL_VERSION.to_string(); assert_eq!( facts.get("session_protocol_version").copied(), - Some("21"), - "the real client must negotiate the v21 panel wire: {text}" + Some(session_version.as_str()), + "the real client must negotiate this binary's wire: {text}" ); assert_eq!( facts.get("baseline_protocol_version").copied(), @@ -880,7 +888,9 @@ fn terminal_mode_keeps_reporting_presence_so_peers_drop_the_stale_caret() { panic!("timed out waiting for {what}"); } - assert_eq!(PROTOCOL_VERSION, 21); + // Tripwire: a wire bump must be a conscious edit here. v22 is + // `LineWrapFacts` (long-lines Stage 3). + assert_eq!(PROTOCOL_VERSION, 22); let daemon = common::daemon::TestDaemon::spawn_with_env_and_init( &[ ("PMACS_INSTANCE_SEMANTIC_RENDER", "1"),