Polish chrome and rework tab-switch repaint
Module renamed github.com/harrybrwn/patterm → github.com/hjbdev/patterm across imports. Chrome: - Palette redrawn with rounded box-drawing borders, accent left-bar for the selected item, dim hints, and a separator-aware footer. - Tab bar grew from 1 row to 3: labels with breathing room, a dim argv subtitle truncated to each tab's width, and an accent thick underline for the focused tab with a faint divider extending across the rest of the host width. Layout, viewport-renderer, and screen- renderer tests updated for the new mainTop. - Sidebar reuses the same palette: accent section headers, `▎` selection marker, `●`/`○` status glyphs, dim previews. - Shared SGR constants moved into internal/app/style.go. Palette input: - Adjacent duplicate arrow events (legacy `\x1b[B` + kitty `\x1b[57353u` for one keypress, or two of the same form) are now collapsed via peekArrowEvent + chunk-level dedupe in processStdin. - On open, push `\x1b[>0u` onto the host's kitty keyboard stack so palette input is in plain legacy mode regardless of what the child pushed (codex/ratatui pushes its own flags which had been leaking to the host). Popped on close. Tab-switch repaint (repaintFocused): - Use the emulator's SerializeVT bytes (with SGR / cursor / DECSTBM / tabstops) instead of plain text, fed through the per-focused viewport renderer so the shifter translates row positions. - Prelude resets host SGR / DECOM / DECSTBM (pinned to viewport) / cursor visibility before the replay, so leftover modes from the previously-focused child don't distort the new snapshot. - Re-emit the saved cursor as a child-space CUP after the serialized bytes so the host cursor lands at the emulator's actual position (overriding DECSTBM's home side-effect and the tabstop-setup CHA sequences) AND the renderer's vr.row/vr.col get re-synced via trackCSI. - cursorShifter now carries childRows and rewrites empty `\x1b[r` to `\x1b[<mainTop>;<mainBottom>r` (host coords) — the default (1,1) shifted to (4,4) was producing a one-row scrolling region that scroll-exploded the replay. - After the snapshot lands, nudge the focused child with a one-row PTY winsize toggle so the kernel emits SIGWINCH and ratatui-style TUIs throw away their diff state and emit a fresh frame. Codex still renders incorrectly after a focus switch; see TODO.md "Switch-back render divergence" for the deep investigation handoff.
This commit is contained in:
122
TODO.md
Normal file
122
TODO.md
Normal file
@@ -0,0 +1,122 @@
|
||||
- [ ] Switch-back rendering is wrong for diff-based TUIs (specifically codex / ratatui). Partial progress; deeper investigation needed — details below in "Switch-back render divergence".
|
||||
- [ ] Killed agents are visible in the command palette. They shouldn't be.
|
||||
- [ ] claude failed to connect to patterm mcp -32601
|
||||
- [ ] codex doesn't show the patterm mcp at all
|
||||
- [ ] opencode doesn't show the patterm mcp at all
|
||||
- [ ] Open agents/processes should appear above the option to open a new one in the palette
|
||||
- [ ] Some sort of macros in the command pallete would be nice, like if i type `sw <query>` it would only show the switch entries. Maybe we should have info text greyed out to show these macros.
|
||||
|
||||
---
|
||||
|
||||
## Switch-back render divergence
|
||||
|
||||
### Symptom
|
||||
|
||||
Switching focus to codex (and back to it again after another tab) leaves
|
||||
codex's input box rendered wrong. The input text and the `›` prompt
|
||||
glyph appear on different rows. Typing more characters in codex makes
|
||||
the box "grow" to 4–5 rows tall even though the content is one short
|
||||
line. Claude (claude-code, ink-based) is mostly fine after the fixes
|
||||
below; codex (Rust/ratatui) is not.
|
||||
|
||||
Initial spawn of codex looks correct. The bug only appears after a
|
||||
focus switch off codex and then back.
|
||||
|
||||
### What's already fixed and committed
|
||||
|
||||
These actually helped; don't undo them blindly.
|
||||
|
||||
1. **`cursorShifter` empty-`\x1b[r` bug** (`internal/app/cursorshift.go`)
|
||||
— `\x1b[r` (reset DECSTBM) was being parsed as `(1,1)` and shifted to
|
||||
`\x1b[4;4r`, producing a one-row scrolling region that scroll-exploded
|
||||
the snapshot. Now rewrites empty params to `\x1b[<mainTop>;<mainBottom>r`
|
||||
in host coords. `cursorShifter` carries `childRows` for this. Test:
|
||||
`TestCursorShifterDECSTBMEmptyResetsToViewport`.
|
||||
|
||||
2. **Host-state reset prelude in `repaintFocused`** (`internal/app/app.go`)
|
||||
— before replaying, write `\x1b[0m\x1b[?6l\x1b[<top>;<bot>r\x1b[?25h\x1b[<top>;<left>H`
|
||||
directly to stdout to clear leftover SGR / DECOM / DECSTBM from the
|
||||
previously-focused child.
|
||||
|
||||
3. **Use `SerializeVT` instead of plain text for the snapshot**
|
||||
(`internal/app/app.go: repaintFocused`) — previously `repaintFocused`
|
||||
used `SnapshotChild` (plain text, no SGR). Now it feeds
|
||||
`SerializeChild` bytes through the per-focused-child viewport
|
||||
renderer, preserving colors and cursor state.
|
||||
|
||||
4. **Re-emit cursor as a child-space CUP through the renderer**
|
||||
— `SerializeVT`'s output order is: content with CRLFs, `\x1b[0m`,
|
||||
cursor CUP, **DECSTBM**, tabstops. DECSTBM has a documented side
|
||||
effect of moving the cursor to the scrolling region's home, and the
|
||||
trailing tabstop setup uses CHA (`\x1b[NG`) which leaves the
|
||||
renderer's internal `vr.col` parked at the last tab-stop column.
|
||||
Without a fixup the host cursor and the renderer's tracking both
|
||||
drift. The current code re-emits the saved cursor as a child-space
|
||||
`\x1b[<R+1>;<C+1>H` through the renderer, so the shifter writes the
|
||||
right host CUP and `trackCSI` updates `vr.row`/`vr.col`.
|
||||
|
||||
5. **`NudgeRedraw` on the focused child after replay**
|
||||
(`internal/app/child.go: NudgeRedraw`, called via `defer` in
|
||||
`repaintFocused`) — toggles PTY winsize by one row and back to force
|
||||
the kernel to emit `SIGWINCH`. Intent: make ratatui throw away its
|
||||
internal "last frame" diff state and emit a full frame. After this
|
||||
change the initial load and the post-interaction state of codex are
|
||||
visually equivalent, but both are still wrong.
|
||||
|
||||
### What's still broken
|
||||
|
||||
After all of the above, codex's input box still draws with the input
|
||||
text and the `›` prompt on different rows, and "asdasdasdasd"-style
|
||||
typing makes the box grow vertically instead of staying single-line.
|
||||
|
||||
Suspected causes, in rough order of likelihood:
|
||||
|
||||
- **The renderer is over-shifting some row-positioning sequence that
|
||||
ghostty's `SerializeVT` emits but I haven't recognised.** Run the
|
||||
probe pattern below to see what bytes go through. Pay special
|
||||
attention to anything that targets rows after the DECSTBM is in
|
||||
place, anything that uses DECOM, and any `\x1bD`/`\x1bM` (IND/RI)
|
||||
which scroll within the region.
|
||||
- **Ratatui's internal "previous_buffer" isn't actually getting reset
|
||||
by `SIGWINCH`** in this PTY environment, or it's getting reset to a
|
||||
size that doesn't match the emulator's. The one-row toggle in
|
||||
`NudgeRedraw` might be a bad idea — try direct `kill(pid, SIGWINCH)`
|
||||
with no size change (the kernel's `TIOCSWINSZ` skips SIGWINCH when
|
||||
the size is unchanged, so we'd need to send the signal explicitly).
|
||||
See `Child.signal` for the helper.
|
||||
- **`childRows`/`childCols` reported via `TIOCGWINSZ` isn't what codex
|
||||
expects.** If codex reads winsize at startup and caches it, our
|
||||
`tabBarRows` change (1 → 3) might have left the cached size stale
|
||||
in some path. Verify by spawning codex fresh after the chrome
|
||||
change and confirming `stty size` inside codex matches
|
||||
`layout.childCols()` × `layout.childRows()`.
|
||||
|
||||
### Investigation tools
|
||||
|
||||
- `internal/vt/probe_test.go` doesn't exist any more; recreate it to
|
||||
print `SerializeVT` output for representative cases. The relevant
|
||||
call is `(*GhosttyEmulator).SerializeVT()`. Confirmed shape:
|
||||
```
|
||||
<content with CRLFs>\x1b[0m\x1b[<r>;<c>H\x1b[<top>;<bot>r\x1b[3g\x1b[NG\x1bH...
|
||||
```
|
||||
- Add a debug tee around `viewportRenderer.Render` to log the raw
|
||||
bytes codex emits **after** the snapshot replay. That will show
|
||||
whether codex is emitting CUPs that target wrong rows (suggesting
|
||||
its diff state is wrong) or whether it's emitting reasonable CUPs
|
||||
and the renderer is mis-shifting them.
|
||||
- The user said they're building a harness so agents can iterate on
|
||||
this without manual screenshotting; once that exists, the diagnose
|
||||
loop is: replay snapshot → capture host stdout → diff against
|
||||
expected. Start with the simplest reproduction: spawn codex, switch
|
||||
away, switch back, type one character, compare host bytes against a
|
||||
golden file.
|
||||
|
||||
### Files touched (so the next agent knows what to read)
|
||||
|
||||
- `internal/app/app.go` — `repaintFocused`
|
||||
- `internal/app/cursorshift.go` — DECSTBM handling, `childRows`
|
||||
- `internal/app/viewport_renderer.go` — plumbing for `childRows`
|
||||
- `internal/app/child.go` — `NudgeRedraw`
|
||||
- `internal/app/cursorshift_test.go` — DECSTBM reset coverage
|
||||
- Probe what `(*GhosttyEmulator).SerializeVT()` emits — that's the
|
||||
source of truth for what we're replaying.
|
||||
Reference in New Issue
Block a user