grid: re-intern the pen on every sweep that compacts the style table #423

Merged

natsukium merged 4 commits from grid/gc-remaps-pen into main 2026-09-25 10:12:32 +09:00

Conversation 0 Commits 4 Files changed 8 +298 -29

natsukium commented 2026-09-25 08:15:20 +09:00

Owner

Copy link

Closes #419

TableGc::maybe_sweep took a bare ScreenBuffer and dropped the compacted flag of its style sweep. The daemon's parse core swept grid.screen_mut() through it, so after a compaction Grid::pen_style kept an id that now named another style or none at all: text printed next lost its truecolor colors, underline color and ISO protection until the program set its SGR again.

Change

• maybe_sweep takes &mut impl Sweepable, a sealed trait implemented for Grid (sweeps through Grid::gc_styles / gc_sizings, which re-intern the pen and remap the open sizing-run handle) and for ScreenBuffer (the client shadow, which holds no handle outside its cells). The sweep methods take a token only felis-grid can construct.
• Grid::screen_mut is removed and the ScreenBuffer sweeps are pub(crate), so no driver outside felis-grid can sweep a grid's screen without its handles.
• Audit: pen_style is the only StyleId held outside cells. SavedCursor (DECSC and the inactive screen's slot) stores Attributes and re-interns on restore; current_sizing_handle is remapped by Grid::gc_sizings. has_iso_protected stays consistent: compact recomputes it and the pen re-intern ORs the pen's flag back in.
• The grid-and-cells explanation now says the pen is re-interned, not rooted.

Tests

• table_gc/tests.rs: text printed after a compacting maybe_sweep keeps the pen set before it (truecolor fg/bg, curly underline with its color, ISO protection honored by EL) on the primary and alternate screen, after DECSC before the sweep, and for the primary pen saved across ?1049h; plus a proptest comparing a grid swept at random points against an unswept one over interleaved pen / print / sweep / DECSC / DECRC / alt-screen steps. The regression test and the proptest fail when Grid sweeps its screen bare.
• pool.rs: the daemon's ParseCore::maybe_gc_tables path keeps Rgb(0,16,0) on text printed after the sweep.

just check passes locally.

Closes #419 `TableGc::maybe_sweep` took a bare `ScreenBuffer` and dropped the `compacted` flag of its style sweep. The daemon's parse core swept `grid.screen_mut()` through it, so after a compaction `Grid::pen_style` kept an id that now named another style or none at all: text printed next lost its truecolor colors, underline color and ISO protection until the program set its SGR again. ## Change - `maybe_sweep` takes `&mut impl Sweepable`, a sealed trait implemented for `Grid` (sweeps through `Grid::gc_styles` / `gc_sizings`, which re-intern the pen and remap the open sizing-run handle) and for `ScreenBuffer` (the client shadow, which holds no handle outside its cells). The sweep methods take a token only felis-grid can construct. - `Grid::screen_mut` is removed and the `ScreenBuffer` sweeps are `pub(crate)`, so no driver outside felis-grid can sweep a grid's screen without its handles. - Audit: `pen_style` is the only `StyleId` held outside cells. `SavedCursor` (DECSC and the inactive screen's slot) stores `Attributes` and re-interns on restore; `current_sizing_handle` is remapped by `Grid::gc_sizings`. `has_iso_protected` stays consistent: `compact` recomputes it and the pen re-intern ORs the pen's flag back in. - The grid-and-cells explanation now says the pen is re-interned, not rooted. ## Tests - `table_gc/tests.rs`: text printed after a compacting `maybe_sweep` keeps the pen set before it (truecolor fg/bg, curly underline with its color, ISO protection honored by EL) on the primary and alternate screen, after DECSC before the sweep, and for the primary pen saved across `?1049h`; plus a proptest comparing a grid swept at random points against an unswept one over interleaved pen / print / sweep / DECSC / DECRC / alt-screen steps. The regression test and the proptest fail when `Grid` sweeps its screen bare. - `pool.rs`: the daemon's `ParseCore::maybe_gc_tables` path keeps `Rgb(0,16,0)` on text printed after the sweep. `just check` passes locally.

natsukium added 3 commits 2026-09-25 08:15:20 +09:00

grid: re-intern the pen on every sweep that compacts the style table ... c0367f3426

The daemon's parse core and the client's shadow both sweep through
TableGc::maybe_sweep, which took a bare ScreenBuffer and dropped the
"compacted" flag its style sweep returned. On the daemon path that left
Grid::pen_style holding an id the compaction had reassigned or cut off,
so the next printed text resolved to another style or to the default
pen: a long truecolor session lost its colors, underline color and ISO
protection until the program set its SGR again. Only Grid::gc_styles
re-interned the pen, and nothing forced the driver to go through it.

maybe_sweep now takes a sealed Sweepable owner instead of a screen.
Grid sweeps through its own gc_styles / gc_sizings, which re-establish
the pen memo and the open sizing-run handle; ScreenBuffer stays
sweepable for the shadow, which holds no handle outside its cells.
Grid::screen_mut is gone and the ScreenBuffer sweeps are crate-private,
so code outside felis-grid cannot sweep a grid's screen without its
handles. Patching the daemon's one call site was rejected: the next
driver would have the same hole.

The saved cursors (DECSC and the inactive screen's slot) hold Attributes
rather than ids and re-intern on restore, so they need no remap; the
new tests pin that too, on both screens.

Assisted-by: Claude Code

grid: keep the Sweepable sweeps callable only by TableGc ... 859b4f1594

Sealing Sweepable stopped outside implementations but left its sweep
methods public, so code outside felis-grid could still compact a bare
ScreenBuffer's tables while holding a StyleId or sizing handle into
them. The sweeps now take a token type that only felis-grid can name.

Assisted-by: Claude Code

docs: say the style sweep re-interns the pen rather than rooting it ... b67a2de2e6

The style-interning section listed the pen_style memo among the sweep's
roots, but the sweep marks only cells and re-interns the pen after a
compaction. The difference is the one the pen fix turns on: a pen set
but not yet printed is on no cell, so a driver that skips the re-intern
leaves it stale.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-25 09:29:42 +09:00

Merge remote-tracking branch 'origin/main' into grid/gc-remaps-pen 8af0437a5a

natsukium merged commit bb3379a38b into main 2026-09-25 10:12:32 +09:00

natsukium deleted branch grid/gc-remaps-pen 2026-09-25 10:12:32 +09:00

natsukium referenced this pull request from a commit 2026-09-25 10:12:33 +09:00

grid: re-intern the pen on every sweep that compacts the style table (#423)

natsukium referenced this pull request 2026-09-25 10:47:31 +09:00

grid: cache recent pens in front of the style dedup index #429

natsukium referenced this pull request 2026-09-25 11:02:19 +09:00

render: walk only the marked rows to prime and shape glyphs #431

natsukium referenced this pull request 2026-09-25 11:04:36 +09:00

grid: let TableGc force a sweep, and sweep the row-cache test through it #432

natsukium referenced this pull request from a commit 2026-09-25 11:04:36 +09:00

grid: let TableGc force a sweep, and sweep the row-cache test through it

natsukium referenced this pull request from a commit 2026-09-25 11:08:55 +09:00

grid: let TableGc force a sweep, and sweep the row-cache test through it (#432)

natsukium referenced this pull request 2026-09-25 13:15:57 +09:00

bench: follow up the 9d1d842f cross-terminal run #405

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!423

No description provided.