Conversation
The maxEntries guard in Sources.Write was inverted: it skipped every write while the source was still under the configured cap, so any positive history-size value disabled history persistence entirely. The in-memory source does no trimming of its own, so this guard is the only cap, and the bug was masked only because the unset default normalises to -1 (unlimited). Use `maxEntries > 0 && Len() >= maxEntries` so a positive cap stops appending once full while unlimited (<= 0) always writes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The default vi-insert keymap binds the down arrow to down-line-or-search, but no such command was registered (only its up-line-or-search sibling and the unrelated down-line-or-select existed), so the binding resolved to nothing and the key did nothing. Add downLineOrSearch, the symmetric counterpart of upLineOrSearch: move down a line within the buffer, or search history forward when already on the last line. A test asserts every arrow-navigation action bound by the default keymaps resolves to a registered command. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
color.Trim ended in input[:maxPrintableLength], which panics when the budget is negative. The completion column math reaches this on very narrow terminals, where the available width is smaller than the trailing padding (maxDisplayWidth - trailingValueLen < 0). Treat a non-positive budget as "nothing fits" and return empty, which guards both group.go call sites at the source. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Terminals deliver line breaks inside a bracketed paste as \r or \r\n. Inserting those raw carriage returns corrupts the line buffer and breaks multiline display and evaluation. Convert them to \n before inserting the pasted text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LineMove applied CheckCommand after every step, clamping the cursor off a line's trailing newline even in emacs and vi-insert modes. Because every LineMove caller already goes through Shell.execute -- which applies CheckCommand in vi-command mode and CheckAppend otherwise -- this double-applied the command-mode rule, leaving the cursor one column short of the end when moving onto a shorter line. Drop the in-loop clamp; LineMove now leaves the end-of-line position and the keymap-aware post-step decides. Tests cover the regression, the emacs/vi-command clamp contract, and sticky-column round trips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The menu-complete-display-prefix option was only half-implemented: with it
set, the menu was displayed but the prefix shared by all candidates was
never inserted, so the option did less than GNU readline documents
("display the common prefix of the list of possible completions before
cycling through the list").
Add Engine.InsertCommonPrefix, which extends the typed word with the
longest prefix shared by every candidate (a no-op when they share nothing
beyond what is typed), and call it from complete-word and menu-complete
when the option is set. Behaviour is unchanged when the option is off (its
default), so this only completes the opt-in path. The shared-prefix
comparison is case-sensitive, never inserting characters not shared
verbatim by all candidates.
Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
Batch of fixes surfaced while reviewing the
gloathub/go-readlinefork's commits-ahead. Each fix is paired with a regression test; the fork's remaining commits were either already implemented in our tree, opinionated default/API changes we declined, or GNU-deviating behavior we keep faithful.Fixes
fix(history): write history again when history-size is set —Sources.Write's cap guard was inverted (maxEntries >= Len()), so any positivehistory-sizeskipped every write. The in-memory source does no trimming of its own, so this guard was the only cap. Corrected tomaxEntries > 0 && Len() >= maxEntries.fix(history): make vi-insert down-arrow navigate again — the default vi-insert keymap binds the down arrow todown-line-or-search, but no such command was registered (onlyup-line-or-searchand the unrelateddown-line-or-select), so the key did nothing. Added the symmetricdownLineOrSearch.fix(color): guardTrimagainst a non-positive budget —Trimended ininput[:n]and panicked on a negative budget, reachable from completion column math on very narrow terminals. Now returns empty for a non-positive budget, guarding both call sites at the source.fix(emacs): normalise carriage returns in bracketed paste — terminals deliver pasted line breaks as\r/\r\n; inserting them raw corrupted the line buffer and multiline rendering. Converted to\nbefore insertion.fix(core): stopLineMovelanding one column short of line end —LineMoveappliedCheckCommandevery step, clamping the cursor off a line's trailing newline even in emacs/vi-insert. Since every caller already routes throughShell.execute(which applies the keymap-aware check), this double-applied the command-mode rule. Dropped the in-loop clamp.Feature (opt-in, no API/default change)
feat(completion): insert common prefix formenu-complete-display-prefix— the option was half-implemented (menu shown, shared prefix never inserted). Completes the GNU-documented semantics. Only runs when the option is set (off by default);InsertCommonPrefixis a method on the internal completion engine, so no public API change.Testing
New tests cover each fix; the
LineMovechange was verified to fail against the old code before the fix. Full suite (incl. the display golden-screen tests),go vet, andgolangci-lintall clean.🤖 Generated with Claude Code