Merge pull request #219 from levineuwirth/full-grid-resync

fix(frontend): honor full_grid so a resize blanks before repainting
This commit is contained in:
Levi Neuwirth 2026-08-06 13:41:32 +00:00 committed by GitHub
commit 218d2e7acd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 740 additions and 11 deletions

View File

@ -225,6 +225,77 @@ 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
**PR #219** — https://github.com/levineuwirth/pmacs/pull/219. **This
block was written with the lane's first commit, before the PR
existed** — the standing correction from #171 and #215 — so the row
below was filled in rather than invented.
- **Branch `full-grid-resync`**, base `githubsucks/main` @ `da56bec`
(the #218 merge). `githubsucks/full-grid-resync` is the
authoritative tip; any edit to this block advances past whatever SHA
it records. Recover with `git fetch githubsucks && git checkout
full-grid-resync`.
- **Framing `docs/full-grid-resync-framing.md` revision 2**, approved
with **Q#FG1 = A**: the sole grid consumer honors the flag by
resetting style, clearing, then applying spans.
- **First of three QoL stages**, from daily-driver use. Stage 2 is GUI
zoom (the machinery exists — `FontMetrics::scale` already derives
every dimension and is driven by an attach message in centi-pixels;
it is unbound and unpersisted). Stage 3 is long-line wrap/scroll,
which is a design round: **no horizontal viewport exists at all**
(`view_left` / `col_offset` / `hscroll` match nothing in `src/` or
`pmacs-gpu/src/`). Separate branches on purpose — Stage 1 is a
contained fix and must not wait behind Stage 3's design.
### What it ships
`FG-INV` moves onto the protocol type, where consumer authors read it:
a `full_grid: true` delta carries only the frame's **non-default**
cells, so a consumer MUST blank its surface first. The rule already
existed — in the doc comment of a **private field** on the producer's
struct (`src/instance_render.rs:36`), which is why the one consumer
never honored it.
`emit_cell_delta` joins the existing pure escape-sequence helpers in
`src/frontend.rs` (`emit_span`, `emit_status_overlay`, …), and
`apply_message` routes through it.
### Why a green suite missed it for so long
Seven tests cover the flag. Every one asserts the **producer sets it**;
none asserted a **consumer acts on it**, and no runtime reader existed
anywhere in the workspace. "Add a test for the flag" had already been
done. This is handoff §5's *enforcement and documentation drift apart
silently* in a second register, and it is why the fix ships the
contract and the consumer together.
### Verification
- Three unit witnesses, all bitten: order (reset → clear → spans),
**empty spans still clear**, and a differential frame clears never.
The empty-spans case discriminates on its own — under the plausible
`spans.is_empty()` early return the order test still passes and only
that one fails.
- PTY acceptance (`tests/full_grid_resync_acceptance.rs`) drives a real
`SIGWINCH`. **The mark is anchored to content, not time**: a
time-based settle cannot work because a settled pmacs screen emits
per-frame bytes forever. Bitten against the original defect: 34,831
bytes after the first painted frame with no `CSI 2 J`.
- **What it does not prove:** the suites assert on raw bytes; there is
no screen model and no `vt100`/`termwiz`/`vte` dependency. This shows
pmacs *emitted* a blank at the right moment, not that the screen
ended correct. A terminal emulator in test deps is a candidate, not
smuggled in here.
### Not in scope
Stages 2 and 3. The `pmacs.terminal` child-PTY `SIGWINCH` path. Any
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.
## Tree primitive (P5) — MERGED as #217; adoption is the open work
**The lane is gone, not the work.** Rule 4 removes a lane after merge,

View File

@ -0,0 +1,331 @@
# Honoring `full_grid` — QoL Stage 1
**Status: revision 2 — APPROVED, IMPLEMENTED, in review as PR #219.**
Q#FG1 was decided **A**: the sole grid consumer honors the flag by
resetting style, clearing, then applying spans. Reported from
daily-driver use:
zooming a terminal with `Ctrl +/-` leaves the TUI visibly broken —
stale glyphs where content should be blank, and dead regions where
content should be.
This is Stage 1 of three. Stage 2 is GUI zoom; Stage 3 is long-line
wrap/scroll. **They are deliberately separate branches** — this one is
a contained correctness fix with a mechanism already established, and
Stage 3 is a design round that touches `view_top`'s missing sibling
everywhere. Bundling them would hold a daily-driver fix behind a
design.
**Revision 2** folds in five review corrections: the invariant belongs
on the protocol type and updating it is in scope (§1.1a, FG-INV); the
PTY witness must be suffix-scoped or the startup clear satisfies it
against a broken build (§5.2); the unit witnesses must include a
`full_grid` message with **empty spans** (§5.1); the standalone TUI
reaches `present_messages` directly and the earlier draft cited only
the attach route (§3); and the claim that the 19 frontend tests assert
on state rather than output was **wrong** — most assert on output,
which makes the proposed seam a continuation of the house idiom rather
than a new one (§5). §6 now cites the concern it serves, not only the
§20 checklist.
---
## 1. The mechanism, established rather than suspected
`src/instance_render.rs:137` builds the full-grid resync by diffing
against a **blank** grid:
```rust
let spans = if self.needs_full_grid {
let blank = vec![Cell::default(); self.next.len()];
diff(&blank, &self.next, self.size.cols, self.size)
} else {
diff(&self.prev, &self.next, self.size.cols, self.size)
};
```
A cell that *should be blank* is equal to its blank counterpart, so it
**produces no span**. The resync therefore carries only non-default
cells — which is exactly what its comment says it intends.
`src/frontend.rs:326` then receives:
```rust
InstanceMessage::CellDelta { spans, .. } => {
for span in spans { emit_span(&mut self.out, span)?; }
}
```
**`full_grid` is destructured away and discarded.** Nothing clears.
So after a font zoom: the terminal reflows its own content, pmacs
repaints only its non-blank cells, and every position pmacs considers
blank keeps whatever the terminal left there. That is both reported
symptoms from one cause — leftovers where pmacs is blank, dead regions
where the terminal's own reflow already lost content.
### 1.1 Why this survived a green suite
The flag is **documented** (`pmacs-protocol/src/message.rs:651`: "the
initial sync sent on fresh attach *or after a resize where the previous
grid is no longer applicable*") and **tested**
`first_frame_is_full_grid_sync`, `resize_reallocates_and_flags_full_grid`,
`instance_message_cell_delta_carries_full_grid_flag`, and four more.
Every one of them asserts the **producer sets the flag**. Not one
asserts a **consumer acts on it**. A workspace-wide search finds no
runtime reader at all: the only other `CellDelta` match outside the
producer and its tests is `pmacs-gpu/src/main.rs:10486`, which uses it
to build a debug label.
### 1.1a The rule exists — in the wrong place
The binding invariant is already written down, in the doc comment of a
**private field** (`src/instance_render.rs:36`):
> *"Remote frontends use this flag to know they must blank their local
> buffer before applying the deltas."*
That is the real contract, and it is stronger than what the protocol
says. `pmacs-protocol/src/message.rs:651` describes `full_grid` only as
*"the initial sync sent on fresh attach … versus an incremental
frame"* — a **label**, from which no consumer obligation follows. A
consumer author reads the protocol type, not a private field on the
producer's struct.
**So updating that protocol documentation is in scope**, and the
invariant is stated here as the thing the code must satisfy:
> **FG-INV.** A `CellDelta` with `full_grid: true` carries **only
> non-default cells**. It is not a picture of the screen; it is a
> picture of the screen's *ink*. A consumer MUST blank its surface
> before applying those spans, or every cell the resync considers blank
> retains whatever was there before.
Writing it on the protocol type is what stops the drift recurring: the
producer's field comment cannot reach whoever writes the next frontend.
This is the handoff §5 lesson landed in #218 — *enforcement and
documentation drift apart silently; only the enforcement is real* —
recurring in a different register. Worth stating plainly in the lane,
because "add a test for the flag" was already done and did not help.
### 1.2 Why it is specifically *zoom*
`src/frontend.rs:166` clears **once**, at startup:
```rust
queue!(me.out, EnterAlternateScreen, Clear(ClearType::All), cursor::Hide, …)
```
So "diff against blank" is correct exactly once — on the fresh-attach
frame, where the screen provably *is* blank — and wrong for every
resize after, where nothing has cleared. The original design is not
careless; it is correct for the case it was written for and was never
extended to the second case its own doc comment names.
A font-size change is the worst version because the terminal reflows
content *in place* rather than dropping it, maximizing the stale glyphs
that survive.
---
## 2. What was eliminated, so the lane does not re-investigate
Three plausible causes were checked and are **not** it:
| hypothesis | verdict | evidence |
|---|---|---|
| Panel/frame geometry goes stale | **no** | `sync_frame_geometry` runs per frame inside `paint_frame` (`src/editor.rs:4412`) against the live `term_size` |
| Render buffers are not reallocated | **no** | `RenderState::resize` reallocates `prev`/`next` and sets `needs_full_grid` |
| `view_top` is not reconciled on shrink | **no** | probed directly: cursor at line 150, painting at 40 rows gives `view_top=113` / cursor row 37; at 8 rows gives `view_top=145` / cursor row 5. Correct both times |
The probe was scratch and is not kept — it proved a negative, and a
test that asserts working behaviour nobody is changing is upkeep
without a customer.
---
## 3. Q#FG1 — consumer honors the flag, or producer emits everything?
**Two designs, and the count of consumers decides it.**
- **A — the consumer clears when `full_grid` is true**, then applies
spans. Protocol meaning: *the resync assumes a blank surface, and the
flag is the instruction to produce one.*
- **B — the producer emits every cell** on a resync, blanks included,
so no consumer needs to change.
The instinct was that A is the trap this project has already hit twice
— the same predicate living in `wait_for_file`, then
`wait_for_published_file`, then a third copy in
`bottom_panel_stage1_acceptance` (R4, R6). Fixing N consumers
independently is how that happens.
**But there is only one consumer.** Both TUI paths land on
`Frontend::present_messages`: the standalone run loop calls it
**directly** (`src/editor.rs:3911`), and the attached TUI reaches the
same method through its own `present_messages` impl
(`src/attach.rs:397`). One implementation, two routes — the earlier
draft cited only the second, which understated how direct the
standalone path is. The GPU frontend is a
**semantic** frontend and never consumes these spans at all — which is
independently consistent with the report that the GUI does not
mis-render on zoom, it simply ignores zoom.
**Recommendation: A.** With one consumer the trap does not apply, and A
is the better answer on the merits:
- The flag **exists solely to be acted on**. Under B it stays unread —
and a flag nobody reads is the defect, not the fix. B would leave
the next reader with the same puzzle plus more bandwidth.
- B ships a full grid of mostly-blank spans over the daemon socket on
every resize, to avoid a one-line clear.
- A makes the protocol's documented sentence true, rather than working
around it.
**Q#FG1 — DECIDED: A**, on approval. The consumer honors the flag;
`full_grid` keeps its sparse-resync meaning and the producer is
unchanged.
*Implementation note, recorded because it confirmed the reasoning
rather than merely following it:* the fix is `emit_cell_delta` beside
`emit_span` and `emit_status_overlay` — no struct change, no generic
parameter, no new pattern. B would have touched the producer, changed
what every resync costs on the wire, and left the flag unread.
---
## 4. Q#FG2 — what "clear" must include
Not just `Clear(ClearType::All)`. The clear paints with the *current*
background, so it must be preceded by `ResetColor` and
`SetAttribute(Attribute::Reset)`, or a resync taken while a styled span
was last emitted will wash the screen in that style.
`present_messages` already brackets its output in
`BeginSynchronizedUpdate` / `EndSynchronizedUpdate` (`src/frontend.rs:223`,
`483`, `502`), so clear-then-repaint lands atomically and does not
flicker. **No new synchronization is needed** — this is why the fix is
small.
---
## 5. Verification
`out` is a concrete `BufWriter<Stdout>` (`src/frontend.rs:126`), so
`Frontend::apply_message` cannot be observed directly. **The seam is
already the house idiom in this file**, though — `emit_span` is a free
function over a writer, and the tests around
`src/frontend.rs:870``1037` capture into `Vec<u8>` and assert on the
exact escape sequences (`src/frontend.rs:587` says so in as many
words: *"Tested below by capturing into a `Vec<u8>`"*).
*An earlier draft said those 19 tests "assert on state rather than
output". That was wrong — most of them assert on output, which makes
the seam a continuation rather than an introduction.*
So: **add `emit_cell_delta` beside the existing helpers** and route
`apply_message` through it. No struct change, no generic parameter, no
new pattern.
### 5.1 Unit witnesses
1. **`full_grid: true` emits reset + clear before any span.** Order
matters and is asserted as order, not membership: a clear *after* a
span erases the frame it was meant to precede.
2. **`full_grid: true` WITH EMPTY SPANS still clears.** This is the
exact stale-blank case and the one a careless implementation
misses — an early return on `spans.is_empty()` is the obvious
"optimization", and it reintroduces the entire bug for the frame
that needs the clear most: a resync to a screen that should be
blank.
3. **`full_grid: false` emits neither**, whatever the spans are.
### 5.1a What implementation changed about §5.2
**The time-based settle described below does not work**, and the
framing said to build it. A settled pmacs screen emits per-frame bytes
indefinitely, so "output stopped growing" never becomes true — the wait
simply runs to its deadline. The vterm suite already recorded the same
behaviour ("a settled screen emits empty diffs forever"); this framing
did not connect it.
The shipped witness anchors its mark to **content** instead: just past
the first painted byte of the fixture. That excludes both of startup's
clears *by construction* rather than by timing — `Frontend::new` clears
before any frame exists, and the first frame is itself a resync whose
clear precedes its own spans — so no timing assumption survives in the
test at all. It is strictly stronger than what §5.2 asked for.
One further correction found by running it: the repaint-ordering
assertion must be scoped to **after the new clear**. The fixture
repeats its marker, so the suffix opens with the tail of startup's own
frame, and an unscoped comparison measures the clear against paints it
was never meant to precede.
### 5.2 PTY acceptance — as originally framed
`tests/common/pty.rs:42` exposes `resize(rows, cols)`, so the real
scenario is reachable: spawn pmacs, put distinctive content on screen,
resize the PTY (a real `SIGWINCH`), and assert on what follows.
**The assertion must be suffix-scoped, or it proves nothing.**
`src/frontend.rs:166` emits `Clear(ClearType::All)` at startup, so a
test that searches the whole output finds a clear **whether or not
resize handling works** — it would pass against the current broken
build. So:
- capture `output().len()` as a mark **only after startup is known
complete** (a startup still in flight would put its own clear into
the suffix and recreate the same false pass, one step later);
- resize;
- slice **strictly after the mark**, and assert that within that
suffix, reset-color + SGR-reset + clear all appear **before the
first repaint output**.
**A limit, stated rather than discovered in review:** the vterm suites
assert on **raw output bytes**; there is no screen model and no
`vt100` / `termwiz` / `vte` dependency in the workspace. So this proves
*pmacs emitted a clear at the right moment*, not *the screen ended up
correct*. Closing that gap means introducing a terminal emulator into
the test dependencies — **out of scope here**, and recorded as a
candidate rather than smuggled in.
---
## 6. Coherence impact (§20 requirement)
**Concern served: `COHERENCE.md` §16, "Productize the Semantic
Frontend Architecture."** §16 names *efficient incremental updates* and
*stable remote attachment* among the properties the semantic protocol
must actually deliver, and FG-INV is a rule the incremental-update
mechanism depends on. A resync that silently fails to resync is that
concern's failure mode, not a cosmetic one.
**No scorecard change.** Row 16 reads **Strong** (`COHERENCE.md:112`),
and this lane neither earns nor forfeits that: it repairs one unhonored
invariant inside a mechanism the row already credits. Per §25 an
audited claim moves with the PR that changes it — nothing here changes
what the row asserts, so it stays put and this sentence records that
the question was asked.
- **Journey steps touched:** none. This is rendering correctness, not
a new capability or step.
- **Interaction islands added:** none. No new keybinding, mode, or
surface.
- **Config registry:** no new settings. Stage 2's persisted zoom level
and Stage 3's wrap/scroll toggle enter it; Stage 1 has no option to
register.
- **Background-work attribution:** none. No async work.
---
## 7. Not in scope
GUI zoom (Stage 2) and long-line wrap/scroll (Stage 3). A terminal
emulator in the test dependencies. Changing what `full_grid` *means*
FG-INV documents the existing contract on the protocol type, it does
not redefine it, and the sparse-resync design stays as is. The `pmacs.terminal` child-PTY
`SIGWINCH` path, which is a different question about a different
process. Any change to *when* `needs_full_grid` is set — the producer's
triggers are correct and verified in §2.

View File

@ -648,15 +648,42 @@ pub enum GoodbyeReason {
/// Rendering and signals from instance to frontend.
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub enum InstanceMessage {
/// Cell deltas. `full_grid = true` is the initial sync sent on
/// fresh attach (or after a resize where the previous grid is no
/// longer applicable); `full_grid = false` is a differential
/// frame.
/// Cell deltas. `full_grid = true` is a resync, sent on fresh
/// attach and after a resize; `full_grid = false` is a
/// differential frame.
///
/// # FG-INV — a resync carries only the frame's *ink*
///
/// **A consumer MUST blank its surface before applying the spans of
/// a `full_grid = true` delta.** This is an obligation, not a
/// label.
///
/// The producer builds a resync by diffing the frame against a
/// **blank** grid, so a cell that should be blank equals its blank
/// counterpart and **produces no span**. A resync is therefore a
/// picture of the screen's *ink*, not a picture of the screen: it
/// says where the non-default cells are and says nothing whatever
/// about the rest. Apply it to a surface that was not blanked and
/// every cell the resync considers empty keeps what was underneath.
///
/// A resync whose frame is entirely blank carries **no spans at
/// all**, and it is exactly the delta that most needs the surface
/// blanked — so "no spans, nothing to do" is wrong.
///
/// This rule previously lived only in a doc comment on a private
/// field of the producer's struct, where no consumer author would
/// read it. The grid TUI consequently ignored the flag: correct for
/// the fresh-attach frame, which follows a `Clear`, and wrong for
/// every resize after, which follows nothing. Zooming a terminal
/// font left the previous frame showing through.
CellDelta {
/// One run of changed cells per `DiffSpan`.
/// One run of changed cells per `DiffSpan`. On a resync these
/// are the frame's non-default cells **only** — see FG-INV
/// above.
spans: Vec<DiffSpan>,
/// Whether `spans` represents a full-grid resync (true on
/// fresh attach or post-resize) versus an incremental frame.
/// Whether `spans` is a resync (fresh attach or post-resize)
/// versus an incremental frame. On `true` the consumer owes
/// the blanking FG-INV describes.
full_grid: bool,
},
/// Cursor position and visibility update.

View File

@ -323,10 +323,8 @@ impl Frontend {
/// are reserved for v0.3 (GUI / multi-frontend) and ignored here.
pub fn apply_message(&mut self, msg: &InstanceMessage) -> io::Result<()> {
match msg {
InstanceMessage::CellDelta { spans, .. } => {
for span in spans {
emit_span(&mut self.out, span)?;
}
InstanceMessage::CellDelta { spans, full_grid } => {
emit_cell_delta(&mut self.out, spans, *full_grid)?;
}
InstanceMessage::Cursor(state) => match state {
Some(cs) if cs.visible => {
@ -581,6 +579,49 @@ impl Drop for Frontend {
// Span emission (pure; testable)
// ---------------------------------------------------------------------------
/// Emit one `CellDelta` — the resync blank, then every span.
///
/// **FG-INV.** A `CellDelta` with `full_grid: true` carries only the
/// **non-default** cells of the frame: the producer diffs against a
/// blank grid (`crate::instance_render::RenderState::render_frame`), so
/// a cell that should be blank produces no span at all. The resync is a
/// picture of the screen's *ink*, not of the screen. A consumer that
/// applies those spans to a surface it has not blanked keeps whatever
/// was underneath every blank cell.
///
/// That is not hypothetical. The startup path clears once
/// ([`Frontend::new`]), which made the sparse resync correct for
/// exactly one frame — the fresh-attach frame, where the screen
/// provably is blank — and wrong for every resize after it. Zooming a
/// terminal font left the reflowed previous frame showing through.
///
/// The clear is preceded by `ResetColor` + `SetAttribute(Reset)`
/// because `Clear` paints with the *current* background: a resync taken
/// while a styled span was last emitted would otherwise wash the screen
/// in that style.
///
/// **Empty spans still clear.** A resync whose frame is entirely blank
/// carries no spans at all, and that is precisely the frame that most
/// needs the surface blanked. Returning early on `spans.is_empty()`
/// would look like an optimization and reintroduce the whole defect.
///
/// Pure and tested below by capturing into a `Vec<u8>`, like its
/// neighbours.
fn emit_cell_delta<W: Write>(w: &mut W, spans: &[DiffSpan], full_grid: bool) -> io::Result<()> {
if full_grid {
queue!(
w,
ResetColor,
SetAttribute(Attribute::Reset),
Clear(ClearType::All)
)?;
}
for span in spans {
emit_span(w, span)?;
}
Ok(())
}
/// Emit a diff span as escape sequences to `w`.
///
/// Pure: the same span produces the same byte output, regardless of any
@ -861,6 +902,74 @@ mod tests {
.expect("the grid frontend must drop StatuslineSegments silently");
}
/// FG-INV, witness 1: a resync blanks the surface before it paints,
/// and the ORDER is the claim — a clear *after* a span erases the
/// frame it was meant to precede.
#[test]
fn a_resync_resets_style_and_clears_before_any_span() {
let span = DiffSpan {
start: CellCoord::new(0, 0),
cells: vec![ch('x')],
};
let mut out = Vec::new();
emit_cell_delta(&mut out, std::slice::from_ref(&span), true).unwrap();
let s = String::from_utf8_lossy(&out);
let clear = s.find("\x1b[2J").expect("resync must clear: {s:?}");
let reset_color = s.find("\x1b[0m").expect("resync must reset: {s:?}");
let glyph = s.find('x').expect("the span is still painted: {s:?}");
assert!(
reset_color < clear,
"reset must precede the clear — `Clear` paints with the CURRENT \
background, so a resync taken mid-style would wash the screen \
in it: {s:?}"
);
assert!(
clear < glyph,
"the clear must precede the paint, or it erases the frame it \
was meant to precede: {s:?}"
);
}
/// FG-INV, witness 2: the frame that most needs blanking carries no
/// spans at all.
///
/// A resync of an entirely blank frame produces zero spans, because
/// the producer diffs against a blank grid. `spans.is_empty()` looks
/// exactly like "nothing to do" — and an early return there
/// reintroduces the whole defect for the one frame whose entire
/// content IS the blanking.
#[test]
fn a_resync_with_no_spans_still_clears() {
let mut out = Vec::new();
emit_cell_delta(&mut out, &[], true).unwrap();
let s = String::from_utf8_lossy(&out);
assert!(
s.contains("\x1b[2J"),
"an empty resync is the stale-blank case, not a no-op: {s:?}"
);
}
/// FG-INV, witness 3: the obligation is the flag's, not the
/// message's. A differential frame that clears would erase
/// everything it does not repaint.
#[test]
fn a_differential_frame_never_clears() {
let span = DiffSpan {
start: CellCoord::new(1, 1),
cells: vec![ch('y')],
};
let mut out = Vec::new();
emit_cell_delta(&mut out, std::slice::from_ref(&span), false).unwrap();
let s = String::from_utf8_lossy(&out);
assert!(
!s.contains("\x1b[2J"),
"a differential frame must not clear: {s:?}"
);
assert!(s.contains('y'), "but it still paints: {s:?}");
}
#[test]
fn emit_span_writes_cursor_move_then_chars() {
let span = DiffSpan {

View File

@ -0,0 +1,191 @@
//! Full-grid resync acceptance — FG-INV at the real PTY boundary.
//!
//! A `CellDelta` with `full_grid: true` carries only the frame's
//! **non-default** cells: the producer diffs against a blank grid, so a
//! cell that should be blank produces no span. A consumer that applies
//! those spans to a surface it has not blanked keeps whatever was
//! underneath every blank cell.
//!
//! The grid TUI ignored the flag entirely. That was correct for exactly
//! one frame — the fresh-attach frame, which follows `Frontend::new`'s
//! `Clear` — and wrong for every resize after it, which follows
//! nothing. Zooming a terminal font left the reflowed previous frame
//! showing through.
//!
//! # Why this test is suffix-scoped, and why it settles first
//!
//! Startup emits its own `Clear(ClearType::All)`. A test that searches
//! the whole output therefore finds a clear **whether or not resize
//! handling works** — it passes against the broken build and proves
//! nothing. So the assertion is confined to bytes emitted strictly
//! after a mark.
//!
//! Taking that mark is the subtle half. Marking while startup is still
//! in flight puts startup's own clear *into the suffix* and recreates
//! the same false pass one step later. The mark is therefore anchored
//! to CONTENT, not to time: it sits just past the first painted byte of
//! the fixture, and both of startup's clears provably precede that —
//! `Frontend::new` clears before any frame exists, and the first frame
//! is itself a resync whose clear precedes its own spans.
//!
//! A time-based settle was tried first and does not work: a settled
//! pmacs screen emits per-frame bytes indefinitely, so "output stopped
//! growing" never becomes true.
//!
//! # What this proves, and what it does not
//!
//! The vterm suites assert on raw output bytes; there is no screen
//! model and no `vt100` / `termwiz` / `vte` dependency in the
//! workspace. This proves **pmacs emitted a blanking sequence at the
//! right moment**, not that the screen ended up correct. Closing that
//! gap needs a terminal emulator in the test dependencies, which is
//! deliberately out of this lane's scope.
use std::time::{Duration, Instant};
#[path = "common/mod.rs"]
mod common;
use common::pty::{PmacsPty, spawn_pmacs_in_pty};
/// CSI 2 J — erase the whole display.
const CLEAR_ALL: &[u8] = b"\x1b[2J";
/// SGR 0 — reset colors and attributes. `Clear` paints with the
/// *current* background, so a resync taken mid-style would otherwise
/// wash the screen in it.
const SGR_RESET: &[u8] = b"\x1b[0m";
fn contains(haystack: &[u8], needle: &[u8]) -> bool {
haystack.windows(needle.len()).any(|w| w == needle)
}
fn find(haystack: &[u8], needle: &[u8]) -> Option<usize> {
haystack.windows(needle.len()).position(|w| w == needle)
}
/// Wait until `needle` has been painted, and return the index just past
/// its first occurrence.
///
/// **That index is the mark, and it is anchored to CONTENT rather than
/// to time.** Startup's blanking provably precedes it: `Frontend::new`
/// clears before any frame exists, and the first frame is itself a
/// resync whose own clear precedes its own spans — so both clears are
/// behind the first byte of painted content, always, with no timing
/// assumption at all.
///
/// A time-based settle was tried first and is wrong here: a settled
/// pmacs screen keeps emitting per-frame bytes indefinitely (the vterm
/// suite records the same thing — "a settled screen emits empty diffs
/// forever"), so "output stopped growing" never becomes true and the
/// wait cannot distinguish a finished startup from a live one.
fn mark_after_first_paint(pty: &PmacsPty, needle: &[u8], timeout: Duration) -> usize {
let deadline = Instant::now() + timeout;
loop {
let out = pty.output();
if let Some(at) = find(&out, needle) {
let mark = at + needle.len();
assert!(
contains(&out[..mark], CLEAR_ALL),
"premise: startup DOES blank the host before painting — \
which is exactly why the assertion below must not be \
allowed to see startup's bytes"
);
return mark;
}
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));
}
}
/// Wait for a blanking sequence to appear strictly after `mark`, and
/// return the suffix containing it.
///
/// Waiting for the *clear specifically* rather than for "any new
/// output" is what makes the timeout meaningful: pmacs emits per-frame
/// bytes regardless, so "output grew" would be satisfied instantly by
/// noise and the assertion would race the resize it is meant to
/// observe.
fn suffix_with_blank_after(pty: &PmacsPty, mark: usize, timeout: Duration) -> Vec<u8> {
let deadline = Instant::now() + timeout;
loop {
let out = pty.output();
if out.len() > mark && contains(&out[mark..], CLEAR_ALL) {
// Let the rest of the frame land so the ordering assertions
// see the whole repaint, not its first fragment.
std::thread::sleep(Duration::from_millis(200));
return pty.output()[mark..].to_vec();
}
if Instant::now() >= deadline {
let out = pty.output();
let suffix = &out[mark.min(out.len())..];
panic!(
"FG-INV: the post-resize resync must blank the host, and \
no CSI 2 J appeared in the {} bytes emitted after the \
first painted frame. Suffix head: {:?}",
suffix.len(),
String::from_utf8_lossy(&suffix[..suffix.len().min(400)])
);
}
std::thread::sleep(Duration::from_millis(20));
}
}
/// Resizing a real PTY makes pmacs blank the host surface before it
/// repaints.
///
/// This is the user-visible bug: `Ctrl +/-` in a terminal changes the
/// font, the terminal reflows and emits `SIGWINCH`, and pmacs repaints
/// only its non-blank cells over content it never cleared.
#[test]
fn a_pty_resize_blanks_the_host_before_repainting() {
let dir = tempfile::tempdir().expect("tempdir");
let file = dir.path().join("marker.txt");
// Distinctive, and long enough that the shrink below genuinely
// changes what fits.
std::fs::write(&file, "ZQXMARKERQZ\n".repeat(40)).expect("write fixture");
let mut pty = spawn_pmacs_in_pty(
&[file.to_str().expect("utf-8 path")],
&[("HOME", dir.path()), ("XDG_CONFIG_HOME", dir.path())],
24,
80,
);
let mark = mark_after_first_paint(&pty, b"ZQXMARKERQZ", Duration::from_secs(20));
// The zoom: same window, different cell geometry.
pty.resize(12, 40).expect("resize the host PTY");
let suffix = suffix_with_blank_after(&pty, mark, Duration::from_secs(20));
let clear = find(&suffix, CLEAR_ALL).expect("waited for it above");
let reset = find(&suffix, SGR_RESET).expect("the blank is style-reset first");
assert!(
reset < clear,
"the reset must precede the clear — `Clear` paints with the \
CURRENT background, so a resync taken mid-style washes the \
screen in it"
);
// …and the blank is followed by the repaint it exists to precede.
//
// Scoped to AFTER the clear on purpose. The mark sits just past the
// *first* painted marker line and the fixture repeats it, so the
// suffix still opens with the tail of startup's own frame —
// comparing the clear against those would compare it against paints
// it was never supposed to precede.
assert!(
contains(&suffix[clear..], b"ZQXMARKERQZ"),
"the resync must repaint after blanking. Without this the test \
would pass on a resize that cleared the screen and painted \
nothing which is the other way to have a broken frame"
);
let _ = pty.write_input(b"\x18\x03"); // C-x C-c
let _ = pty.wait_for_exit(Duration::from_secs(5));
}