Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the TUI startup/teardown behavior to (1) restore bracketed paste mode to its prior state instead of always disabling it on exit, and (2) avoid blocking at startup on terminals that never answer escape-sequence queries, while also doing some related comment/test cleanups.
Changes:
- Add a startup query round-trip to detect whether bracketed paste mode was already enabled, and restore it accordingly on exit.
- Add a bounded wait before the first read for terminal query replies to prevent startup hangs on non-responding terminals.
- Update comments/tests and document the behavioral changes in the changelog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tui/light.go | Adds startup query plumbing, bracketed paste state tracking, and restores paste mode on exit. |
| src/tui/light_unix.go | Implements query batching + timeout-bounded read for terminal replies; parses cursor/paste replies. |
| src/tui/light_windows.go | Adds a Windows stub for the new startup query API (cursor offset only). |
| src/util/chars.go | Comment wording adjustments around foldable rune range rationale. |
| src/util/chars_test.go | Removes a verbose/unclear tab wrap block; tightens comment wording. |
| src/algo/runeprefilter_test.go | Comment wording adjustments (“pins” → “verifies/checks”, clarity tweaks). |
| src/algo/fastpath_equiv_test.go | Comment wording (“oracle” → “reference”). |
| src/algo/algo.go | Comment wording adjustment. |
| CHANGELOG.md | Documents bracketed paste restoration and the startup hang fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/tui/light_unix.go:126
- waitReadable returns true on unix.Select error, which can reintroduce the startup hang this code is trying to avoid (a subsequent blocking read may wait for user input). It's safer to treat select errors as "not readable" and let the caller stop waiting / fall back.
if err != nil {
// Fall through to the read and let it report the failure
return true
}
src/tui/light_unix.go:150
- queryTerminal calls getBytesInternal with nonblock=true on retries (tries > 0). If there's no data available, getBytesInternal can still append byte(c) (often 0x00) to the buffer even when the nonblocking read fails, which can corrupt the buffered terminal reply and prevent the regex from ever matching. Guard retries with waitReadable and skip the read when nothing is ready.
buffer, _, err = r.getBytesInternal(false, buffer, tries > 0)
src/tui/light.go:41
- pasteModeRegexp allows an optional leading NUL ("\x00?") but pasteModeRegexpBegin does not. If the terminal includes that NUL in the DECRPM reply, escSequence won't recognize it as a terminal reply and may emit a CtrlSpace (0x00) event before the escape sequence, corrupting input.
var pasteModeRegexpBegin = regexp.MustCompile("^\x1b\\[\\?2004;[0-4]\\$y")
Ask the terminal whether the mode is already on (DECRQM) and put it back that way on exit. Forcing it off broke pasting in shells that run fzf from a line editor widget, which enable the mode only when the editor starts. Terminals that do not answer fall back to disabling it. - Startup queries go out in one write, cursor position last. Every terminal answers DSR, so its reply bounds the wait: a paste reply still missing by then means the terminal does not know the query. - Bound the first read with select(2). Terminals that never answer escape sequences, such as FreeBSD virtual terminals, blocked startup until a key was pressed, and that keystroke was then discarded. Fix #4887 Fix #2860 Fix #976
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.
Contribution Policy
We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement