Conversation
emacsKeys overrode the GNU default \C-h (backward-delete-char) with backward-kill-word. Many terminals send ^H for Backspace, so Backspace deleted a whole word. Drop the override so \C-h falls through to the default backward-delete-char; add a regression test asserting the resolved bind. Ported from moloch--/readline (Will DeDominico / Moriz82). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh and the other frame entrypoints emitted dozens of separate writes to stdout (hide cursor, moves, clears, prompt, line, highlights, helpers, show cursor). A terminal can show those partial frames as flicker, and each is a syscall. Add a buffering layer in internal/term (BeginBuffer/EndBuffer/FlushBuffer/ WriteString/Printf): render entrypoints wrap their body so a whole frame coalesces into a single write. All fmt.Print rendering sites route through term.WriteString; term's own MoveCursor*/bracketed-paste helpers buffer for free. The one ordering constraint: GetCursorPos must see prior output on screen before it reads the reply, so it calls term.FlushBuffer() before the ESC[6n query (unix) / console cursor read (windows). Output goes to stdout, kept distinct from termFile (stderr) used for size/cursor queries. Buffering changes only WHEN bytes are written, never which: the PTY golden-screen tests pass byte-identical. Adds term buffer unit tests (coalescing, nesting, mid-frame flush). Inspired by moloch--/readline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Evaluation of removing the per-keystroke sort in matchBind: the default emacs keymap has 15 sequences that ConvertMeta collapses to the same key bytes with DIFFERENT actions (e.g. ESC-prefixed meta binds overlapping self-insert). The ordering makes the exact match deterministic; dropping it would make keybind resolution depend on Go's randomized map iteration. These tests assert the collisions exist (so the ordering is justified) and that resolution stays deterministic across runs — a guard against a naive 'optimization' that removes the sort. (The real per-keystroke cost is ConvertMeta over every bind, not the sort; addressing that is separate future work.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
Three commits, all GPG-signed. Ideas adapted from / evaluated against the
moloch--/readlinefork.Fixes
fix(keymap)— In emacs mode\C-hwas overridden tobackward-kill-word; many terminals send^Hfor Backspace, so Backspace deleted a whole word. Dropped the override so\C-hfalls through to the GNU defaultbackward-delete-char. Regression test asserts the resolved bind.Performance
perf(display)— Buffer a whole render frame and flush it in a single write.Refresh()(andAcceptLine/RefreshTransient/ClearHelpers) previously emitted dozens of separate stdout writes per keystroke (hide cursor, moves, clears, prompt, line, highlights, helpers, show cursor); a terminal can show those partial frames as flicker, and each is a syscall. Newinternal/termbuffering layer (BeginBuffer/EndBuffer/FlushBuffer/WriteString/Printf); all render-pathfmt.Printsites route through it.GetCursorPoscallsterm.FlushBuffer()before theESC[6nquery (unix) / console cursor read (windows), so the position is measured against on-screen output, not buffered bytes.Test-only
test(keymap)— Guards thatmatchBind's ordering is load-bearing: the default emacs keymap has 15 sequences thatConvertMetacollapses to the same key bytes with different actions, so removing the per-keystroke sort (as the fork did) would make keybind resolution depend on map iteration order. Asserts the collisions exist and that resolution stays deterministic.Evaluated but NOT adopted
ConvertMetarecomputed over all binds per keystroke, not the sort).ensureInputSpacemaintainsstartRowsacross scrolls) but has an untested invalidation gap:WatchResizedoesn't mark the cache dirty, so a terminal resize would mis-render. Not worth the Incorrect cursor/character rendering in terminal prompt #98-class risk, especially now that buffering cuts the probe's cost.Verification
go build,go test ./...(incl. display golden-screen +-race), both unix and Windows builds, andgolangci-lint run→ 0 issues.