Skip to content

fix(tui): keep copy label background under "copied" flash - #3797

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:fix-copied-flash-background
Jul 23, 2026
Merged

fix(tui): keep copy label background under "copied" flash#3797
dgageot merged 1 commit into
docker:mainfrom
dgageot:fix-copied-flash-background

Conversation

@dgageot

@dgageot dgageot commented Jul 23, 2026

Copy link
Copy Markdown
Member

When a user clicks a copy button, the label briefly swaps to a "copied" confirmation. For code blocks, the background of that flash was already forced to match the code block's band. For user-message copy labels, which sit on the BackgroundAlt band, no such fix existed, so the transient label rendered with a transparent background and punched a visible hole in the colored band.

The fix introduces backgroundAt(line, col), which walks the ANSI SGR escape sequences of a rendered line and returns the background color in effect at the given column — handling truecolor, 256-indexed, basic, and bright colors, plus resets. The "copied" flash now calls this helper for all copy labels, replacing the code-block-only special case with a single, general path.

New regression tests cover the user-message band case, the code-block band case, and a table-driven suite for backgroundAt itself.

The transient "copied" confirmation now derives the background in effect
at the label position from the rendered line (SGR parsing) instead of
hard-coding the code-block band, so user-message copy labels keep their
background band too.

Assisted-By: Claude <noreply@anthropic.com>
@dgageot
dgageot requested a review from a team as a code owner July 23, 2026 09:34

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🔴 CRITICAL

1 high-severity finding in new code.

var bg color.Color
var state byte
for w := 0; line != "" && w <= col; {
seq, width, n, newState := ansi.DecodeSequence(line, state, p)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] backgroundAt can spin in an infinite loop when ansi.DecodeSequence returns n == 0

ansi.DecodeSequence can return n == 0 when the first byte of the remaining input is an invalid UTF-8 sequence (e.g., an isolated continuation byte 0x800xBF). In that case:

  • line = line[n:] is a no-op — line does not shrink
  • line != "" remains true
  • w never advances (the invalid byte has visual width == 0)
  • The loop condition w <= col stays satisfied

This causes a busy infinite loop, freezing the TUI.

From charmbracelet/x/ansi@v0.11.7/parser_decode.go, the "Invalid UTF-8 sequence" return path is:

// Invalid UTF-8 sequence
return b[:i], 0, i, NormalState  // i == 0 on first byte → n == 0

This path is reachable whenever a rendered line contains an isolated continuation byte or other invalid UTF-8. While charmbracelet's own rendering is unlikely to emit such bytes, backgroundAt is called on arbitrary terminal lines, so robustness matters.

Fix: add an early-out guard immediately after the DecodeSequence call:

Suggested change
seq, width, n, newState := ansi.DecodeSequence(line, state, p)
seq, width, n, newState := ansi.DecodeSequence(line, state, p)
if n == 0 {
break
}
Confidence Score
🟡 moderate 57/100

@dgageot
dgageot merged commit ec7ef87 into docker:main Jul 23, 2026
11 of 12 checks passed

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants