Conversation
Add a "readline features" command group and a passive hint provider to the
example console, exercising the new reeflective/readline APIs:
- setupReadlineHints: registers rl.Hint.SetProvider, a passive hint computed
from the current line (shows the matched command's short description).
- notify: pushes async status updates into the transient hint lane from a
goroutine (rl.Hint.SetTransient/ClearTransient) — the idle shell repaints
on its own via the async-refresh wake.
- hint set/clear: set or clear a sticky transient hint synchronously.
- scan: async completions — a background "discovery" grows the candidate set
and calls rl.RefreshCompletions() to rebuild the open menu in place, so
hosts appear live with no keystroke.
The feature commands are registered after the generic file-completion loop so
their custom (async) completers are not overwritten. Includes cursor.cast, a
screencast used while diagnosing the hint-area drift bug.
Note: builds against the local readline via the go workspace; needs a readline
release + go.mod bump to build standalone.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upgrade direct dependencies: - reeflective/readline v1.1.4 -> v1.2.0 (fixes the bottom-of-terminal prompt overlap reported in #78, via the NewlineBefore/After fix) - carapace-sh/carapace v1.7.1 -> v1.11.6 - spf13/cobra v1.8.1 -> v1.10.2 - spf13/pflag v1.0.6 -> v1.0.10 - mvdan.cc/sh/v3 v3.7.0 -> v3.13.1 - golang.org/x/exp -> latest The go directive moves to 1.25.0, required by both readline v1.2.0 and mvdan.cc/sh v3.13.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Concurrency: - isExecuting was written under RLock and read with no lock at all; make it an atomic.Bool so all accesses are correct and lock-free. - Several maps/slices (filters, interruptHandlers, histories, cmds) were mutated under RLock, and HideCommands took no lock at all. Writers now take Lock; readers (Menu, ActiveFiltersFor) take RLock. - activeMenu() iterated the menus map with no lock. Cache the active menu in a pointer maintained under Lock, and read it under RLock (also drops the per-call linear scan). - handleInterrupt ranged the handler map unlocked while a handler could mutate it; snapshot matching handlers under RLock, then run them after releasing. Deadlock: - ActiveFiltersFor held a write lock and recursed into itself while holding it (latent self-deadlock on the non-reentrant mutex, plus per-command render contention). Snapshot the filters once under RLock, then walk the command tree lock-free. - SwitchMenu now releases c.mutex before resetPreRun, which reacquires it. Signal leak: - monitorSignals registered a channel via signal.Notify on every command execution and never released it, leaking a channel per command for the process lifetime. execute now defers signal.Stop(sigchan). Highlighting: - A command invoked through an alias was never highlighted because the alias branch broke out of the loop before the highlight block. Fold the alias check into cmdFound. Tests: - concurrency_test.go stresses the shared state from many goroutines and passes under -race. - internal/line/highlight_test.go covers alias highlighting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Drop the unused completion.UnescapeValue; the live completion path uses line.UnescapeValue (completer.go). - Remove the leftover `// menu := app.activeMenu()` comment in the prompt. - Simplify the BindPrompt primary wrapper to a plain nil-guard and fix its stale comment: newline handling now lives in readline, not here. - Drop the redundant `buf == ""` branch (len(buf) == 0 already covers it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Table-driven coverage for the shell-parsing core: internal/line: - Parse: comment stripping, quoting, whitespace collapse, unterminated quotes (error). - Split: words, quotes, escaped spaces, and the unterminated quote/escape remainder paths. - AcceptMultiline: complete vs. unterminated-quote/escape continuation. - IsEmpty, TrimSpaces, UnescapeValue. internal/completion: - splitCompWords: words/quotes plus remainder + error on unterminated quote/escape. - adjustQuotedPrefix: the quote/escape prefix derivation. - sanitizeArgs: newline/tab/escaped-space normalization. - SplitArgs: empty line, partial word, trailing-space new word, unterminated-quote prefix handling, and ANSI color stripping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two hot paths run on (nearly) every keystroke. Reduce their per-call cost without changing observable behavior: - Syntax highlighting: memoize the last result keyed on the input string. readline calls the highlighter on every render (including cursor-only moves), so an unchanged line no longer re-splits the input and re-walks the command tree. The cache is invalidated whenever the command tree is regenerated (Menu.regenerate), so a stale tree can't yield a stale highlight; the key is stored/loaded via atomic.Pointer to stay race-free. - Completion reset: complete() previously called the full resetPreRun() (which rebuilds the command tree, re-binds the prompt, and resets the output buffer) and then redundantly re-hid filtered commands. Split out a lighter resetCommands() that only regenerates + re-filters the tree: the prompt is already bound and no command output is produced during completion, so that extra work was wasted per keystroke. resetPreRun now delegates to the same shared regenerate() core. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cast recording is no longer used by the example app.
… cache - internal/line: HighlightCommandFlags (flags colored, operands passed through). - internal/strutil: Template rendering, the trim func, and ranging. - console: ActiveFiltersFor and CheckIsAvailable, including parent-subtree filter inheritance and activation/deactivation via Hide/ShowCommands. - console: highlight-cache memoization correctness and its invalidation on command-tree regeneration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cel docs - Interrupt matching: handleInterrupt now matches the incoming error against registered handlers with errors.Is first (so sentinel and wrapped errors work), then falls back to message comparison for the historical errors.New(...) pattern. Resolves the long-standing TODO in interrupt.go. - Per-menu newline behavior: NewlineBefore/After/WhenEmpty and EmptyChars are console-wide defaults that a menu can now override via SetNewlineBefore, SetNewlineAfter, SetNewlineWhenEmpty and SetEmptyChars (nil/unset inherits the console value). The pre/post-run display and TransientPrintf resolve through the active menu. - Configurable trapped signals: new Console.Signals field selects which OS signals cancel a running command. It defaults to SIGINT/SIGTERM/SIGQUIT (shared defaultTrapSignals), and monitorSignals registers exactly that set. - Cancellation model: StartContext now documents how a trapped signal cancels the command's cmd.Context(), and that a long-running command must observe that cancellation itself (cobra cannot preempt it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- handleInterrupt: errors.Is (wrapped), message fallback, and no-match. - Menu newline/empty-char overrides: inheritance, override precedence, and clearing back to inheritance. - Console.Signals: default set is populated, and monitorSignals honors a custom set (SIGUSR1 raise, unix-only build tag). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Capture os.Stdout and assert the displayPreRun/displayPostRun newline output across the enabled × when-empty × (empty/non-empty input) matrix, plus a case proving a per-menu newline override is honored by the display path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The module now requires go 1.25.0 (readline v1.2.0 and mvdan.cc/sh v3.13.1), so the CI's pinned Go 1.21 would fail to build. Bump both the unix and windows jobs to Go 1.25. 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
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.
Summary
Upgrades dependencies (notably readline v1.2.0, which fixes the bottom-of-terminal prompt overlap from #78), fixes a set of concurrency/correctness bugs, trims two hot paths, adds four feature improvements, and takes the package from 0 tests to 101 passing cases (all green under
-race).Built on top of the existing
devwork (theexample:commit demonstrating readline hint lanes was already ondev).Dependencies
reeflective/readlinev1.1.4 → v1.2.0 (fixes Prompt string overlaps with input at the bottom #78 prompt overlap)carapacev1.7.1 → v1.11.6,cobrav1.8.1 → v1.10.2,pflagv1.0.6 → v1.0.10,mvdan.cc/sh/v3v3.7.0 → v3.13.1,golang.org/x/exprefreshedgodirective → 1.25.0 (required by readline v1.2.0 and mvdan.cc/sh v3.13.1)go.ymlto Go 1.25 to match (1.21 would no longer build)Bug fixes
monitorSignalsregistered a channel viasignal.Notifyon every command and never released it; nowdefer signal.Stop.isExecutingis now anatomic.Bool; maps/slices (filters,interruptHandlers,histories,cmds) that were mutated underRLock(or no lock) now useLock;activeMenu()no longer reads the menus map unlocked (cached pointer).ActiveFiltersFortook a write lock and recursed while holding it; now snapshots filters underRLockand walks lock-free.SwitchMenureleases the lock beforeresetPreRun.breakskipped the highlight branch).Performance
resetCommands(no prompt rebind / output reset / redundant re-filter per keystroke).Features
errors.Isfirst, message-equality fallback for the historicalerrors.New(...)pattern.StartContext(trapped signal cancelscmd.Context(); a long-running command must observe it).Menu.SetNewlineBefore/After/WhenEmpty/SetEmptyChars(unset → inherits the console default).Console.Signals(defaults to SIGINT/SIGTERM/SIGQUIT).Tests (new — 101 cases, all green under
-race)internal/line(Parse/Split/AcceptMultiline/IsEmpty/…) andinternal/completion(SplitArgs/splitCompWords/adjustQuotedPrefix/sanitizeArgs).strutil.Template, menu filtering (ActiveFiltersFor/CheckIsAvailable+ inheritance), highlight-cache invalidation.-raceconcurrency stress test.Verification
go build ./...,go vet ./..., andgo test -race ./...all pass locally.Closes #78.
🤖 Generated with Claude Code