fix(terminal): let Ctrl+G reach the shell when search is closed - #209
Conversation
The two tests that mount a terminal each carried their own copy of the same ~35-line stub of the Terminal and addon surface useTerminal touches, so an xterm API change had to be taught to both. A third copy was about to land with the Ctrl+G regression test. One fixture now owns the fake. Each test still declares its own vi.mock calls — those are hoisted per file — but they point at the shared classes instead of redefining them. Two additions the copies lacked: getSelection(), which the search controller calls on open, and a SearchAddon that records its finds, since the real one reports hits through onDidChangeResults and a stub cannot.
Ctrl+G never reached the PTY, so bash could not abort a Ctrl+R reverse-i-search: readline's abort needs ^G (0x07) and the byte was never sent. Every shell and TUI use of the key was dead the same way. The custom key handler returned false for Ctrl+G unconditionally — the return sat outside the check for an open search widget. Returning false makes xterm skip the key entirely, encoding nothing, so with the widget closed the branch did no work and still swallowed the key. It now only claims the chord while the widget is open. On the pass-through xterm marks ctrl+letter cancel:true and calls preventDefault itself, so the webview's native find-next — the reason 8eda22a blocked the key in the first place — stays suppressed. Verified against xterm 6.0.0 in a browser: with the widget closed onData receives a single 0x07 and the event ends up defaultPrevented; with it open xterm bails out early and the event still reaches the window handler, which is what cancels the native dialog on that path. The chord match and the next/prev drive were duplicated between the terminal handler and the window handler; both now call one pair of helpers. Fixes VoltiusApp#208
xterm's _keyDown returns early on a false verdict from the custom key handler without calling cancel(), so a chord the handler claimed still bubbled to useKeyboard's window listener. With the search widget open and the canvas focused both handlers ran next()/prev(), and one press moved two hits. The canvas handler now takes the event itself when it drives the widget. It is the right owner: it has this pane's session id, where the window listener keys off activeSessionId. The pass-through path is unchanged — it must leave the event alone so xterm can encode ^G and cancel it on the way out.
Ctrl+F had the same double-fire as Ctrl+G: the canvas handler opened the widget and returned false, xterm returned early without cancelling, and useKeyboard then ran its own Ctrl+F branch — focusing the right panel's search bar over the widget that had just opened. Its comment already claimed useTerminal wins when the canvas has focus; now it does. These two are the only branches that can double-fire. Every other shortcut useKeyboard shares with the canvas sits below its `isInput` guard, and the canvas is xterm's helper textarea. Extracted claimChord() rather than write preventDefault/stopPropagation twice, and renamed the test file, which is no longer only about Ctrl+G.
|
Reviewed, and the diagnosis holds up — I checked the claims about I've pushed two commits onto the branch rather than sending you round again for something this small.
The other half of that if(this._customKeyEventHandler&&!1===this._customKeyEventHandler(e))return!1;It returns early without calling The canvas handler now takes the event when it drives the widget. It is the right owner: it has the pane's own session id, where the window listener keys off
Both call one Worth writing down, because it bounds the bug: these two are the only branches that can double-fire. They are the two placed above I renamed Local runs: |
Fixes #208.
Problem
Ctrl+Gnever reached the PTY, sobashcould not abort aCtrl+Rreverse-i-search — readline's
abortneeds^G(0x07) and the byte was neversent.
Ctrl+Rworked, then the key was simply dead, which is what made it looklike a
Ctrl+G-specific fault. Every other shell/TUI use of the key went thesame way: emacs
keyboard-quit, nano's help toggle, and so on.useTerminal.ts's custom key handler returnedfalseforCtrl+Gunconditionally — the
returnsat outside the check for an open search widget:Returning
falsemakes xterm skip the key entirely, encoding nothing, so withthe widget closed the branch did no work and still swallowed the key. Reported
on Fedora 44 / v0.31.1, but the path has no platform branch.
Fix
The chord is now only claimed while the widget is open. On the pass-through
xterm marks
ctrl+lettercancel: trueand callspreventDefault()itself, sothe native webview find-next — the reason 8eda22a blocked the key, and not a
problem I wanted to reintroduce — stays suppressed.
Measured against the real
@xterm/xterm@6.0.0in a browser, driving actualControl+gkeystrokes rather than synthetic ones:onData[0x07]defaultPrevented: truedefaultPrevented: falseuseKeyboardcancels it thereSo both paths keep the native dialog shut, by different routes: xterm's own
cancel on the shell path, and the existing window handler on the widget path.
The reporter has also confirmed it by hand on the Fedora 44 / WebKitGTK build
this was found on:
Ctrl+Gaborts a reverse-i-search again, and theCtrl+Fwidget still walks its hits.
Tests
src/hooks/useTerminal.ctrlG.test.tsxcovers the handler's verdict for closedwidget, open widget (including that
keyupstays consumed without moving thehit again), and closed-again. Two of its three cases fail against the code this
replaces.
pnpm test— 522 files / 3991 tests pass.pnpm buildclean. No Rust touched.The first commit
The two existing tests that mount a terminal each carried their own copy of the
same ~35-line xterm stub, and the regression test would have been a third. Per
the DRY note in
CLAUDE.mdI extracted one fixture instead of adding a copy.It's a separate commit and drops cleanly if you would rather review a one-file
diff — the fix itself does not depend on it beyond using the fixture.