Repository navigation
fix(coding/tui): keep the cached tool preview read-only while rendering - #35
Merged
Merged
Conversation
A compact tool block renders its rows by prefixing and styling each line, and the rows it was handed were the activity's own preview slice. Writing them back meant every re-render added another branch prefix and left the previous render's colour escapes inside the text, so switching the theme during a session grew a run of └ markers in front of the same diff line. The tone lookup made it worse: it reads the leading characters of the row to tell an added line from a removed one, and a row that starts with an escape fell through to the muted tone, which is why the - and + colours stopped following the theme after the first re-render. toolActivityRows now documents that the caller owns its result and clones a preview before returning it, so the cached activity stays input. The regression test renders one patch under the dark theme and then the light one: it asserts the activity is unchanged, that exactly one branch prefix is applied per render, and that each render carries its own theme's error and added tones with none of the other theme's escapes.
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
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.
A compact tool block renders its rows by prefixing and styling each line, and the
rows it was handed were the activity's own preview slice. Writing them back meant
every re-render added another branch prefix and left the previous render's colour
escapes inside the text, so switching the theme during a session grew a run of └
markers in front of the same diff line. The tone lookup made it worse: it reads the
leading characters of the row to tell an added line from a removed one, and a row
that starts with an escape fell through to the muted tone, which is why the - and +
colours stopped following the theme after the first re-render.
toolActivityRows now documents that the caller owns its result and clones a preview
before returning it, so the cached activity stays input. The regression test renders
one patch under the dark theme and then the light one: it asserts the activity is
unchanged, that exactly one branch prefix is applied per render, and that each render
carries its own theme's error and added tones with none of the other theme's escapes.
Evidence
Before the fix, two renders of one patch under two themes left the activity holding
�[38;2;139;148;158m └ - (13, 13), (31, 61),�[mand then�[38;2;87;96;106m └ �[38;2;139;148;158m └ - ...— one branch prefix per render, with the first theme's escapes inside the second theme's output.After: the activity is byte-identical across renders, one prefix per render, and the dark and light renders each carry their own
errorandidletones.TERM=xterm-256color go test ./...(89 packages),go test -race ./internal/coding/tui/, the three golden digests andgolangci-lint run --new-from-rev=HEAD~ ./...(0 issues) are all clean.