Skip to content

Simplify color detection#16

Merged
kevinmcconnell merged 2 commits intomainfrom
ansi-seq-leak
Mar 18, 2026
Merged

Simplify color detection#16
kevinmcconnell merged 2 commits intomainfrom
ansi-seq-leak

Conversation

@kevinmcconnell
Copy link
Copy Markdown
Collaborator

When detecting terminal colors at startup:

  • Ensure we consume the sentinel marker, so as not to leak it back to the terminal on exit
  • Simplify the logic for processing the response: we can just read and parse the incoming buffer until we get the sentinel, and close the reader on timeout

Copilot AI review requested due to automatic review settings March 18, 2026 12:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors terminal palette detection to read/parse escape sequences incrementally until the DA1 “sentinel” is received (or a timeout closes the TTY), reducing buffering/coordination complexity and ensuring the sentinel response is consumed before exiting.

Changes:

  • Replaced channel/goroutine-based buffering with a detector that reads OSC/CSI sequences directly from a bufio.Reader.
  • Updated detection flow to always wait for the DA1 sentinel (or stop on read error) and to close the TTY on timeout via a single cleanup path.
  • Reworked tests to validate the new incremental reader behavior and the updated detectFrom signature.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/ui/palette_detect.go Implements the new incremental detector (OSC/CSI parsing), and changes timeout handling to close/restore the TTY via a single cleanup function.
internal/ui/palette_detect_test.go Updates tests to exercise readNext/detector behavior and adjusts detectFrom call sites for the new API.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

})
defer cleanup()

time.AfterFunc(timeout, cleanup)
@kevinmcconnell kevinmcconnell merged commit 339f4af into main Mar 18, 2026
8 checks passed
@kevinmcconnell kevinmcconnell deleted the ansi-seq-leak branch March 18, 2026 16:33
kevinmcconnell added a commit that referenced this pull request Mar 27, 2026
This seems to be failing on some terminals; reverting while we
investigate further.

This reverts commit 339f4af, reversing
changes made to e59a942.
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.

2 participants