Rebuild the TTY progress renderer on a model/layout/screen split - #14051
Open
ndeloof wants to merge 2 commits into
Open
Rebuild the TTY progress renderer on a model/layout/screen split#14051ndeloof wants to merge 2 commits into
ndeloof wants to merge 2 commits into
Conversation
…t/screen split The TTY renderer accumulated display-corruption fixes (truncation of details, then progress sizes, then task ids; timer alignment; rune-based measurement) that each patched one symptom of the same structural gap: nothing guaranteed a rendered line fits the terminal, and once a line wraps, cursor arithmetic desyncs and the block corrupts. Replace it with three separable units: - tty_model.go: pure event reducer with an injected clock, preserving first-parent-wins updates, monotonic progress and header counters - tty_layout.go: pure (model, size, now) -> lines function; all widths are measured in terminal cells (go-runewidth, so CJK is correct) on plain text before coloring, and every line is clipped to the terminal width by construction, status text included - tty_screen.go: diff-based repaint; unchanged rows are skipped, identical frames write nothing, a frame is a single Write; a shrinking terminal abandons the block instead of moving the cursor over reflowed rows The writer coordinates them behind a mutex and stops the refresh goroutine through context cancellation, so Done cannot block when the operation context was cancelled first (Ctrl-C during pull). The spinner frame is derived from the clock instead of advancing on every call, and truncation can no longer split multi-byte runes. Visual output is unchanged: the snapshot test reproduces the previous renderer's golden output character for character. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
…ther row The "[+] op N/M" header was the one line bypassing renderSegs, so on a very narrow terminal it could wrap and desync the cursor arithmetic the rest of the design guarantees against. Route it through the same clip and cover degenerate widths (8, 12 cells) in the invariant test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Replaced the TTY progress renderer with a three-layer design that makes the recurring display corruptions impossible by construction:
tty_model.go— pure event reducer (injected clock); preserves first-parent-wins updates, monotonic progress and header counterstty_layout.go— pure(model, size, now) → linesfunction; widths measured in terminal cells (go-runewidth, CJK-correct) on plain text before coloring; every line — status text included — is clipped to the terminal width, so lines can never wrap and desync cursor arithmetictty_screen.go— diff-based repaint: unchanged rows are skipped, identical frames write nothing, oneWriteper frame; terminal shrink abandons the block instead of repainting over reflowed rowsThe writer coordinates them behind a mutex; the refresh goroutine stops through context cancellation, so
Done()can no longer deadlock when the operation context was cancelled first (Ctrl-C during pull). The spinner frame derives from the clock instead of advancing per call, and truncation can no longer split multi-byte runes.Visual output is unchanged — the snapshot test reproduces the previous renderer's golden output character for character. Net −401 lines.
Related issue
Structural follow-up to the line-overflow / cursor-desync family (#13595) and the
Done()deadlock (#13639).🤖 Generated with Claude Code