Fix non-hardcoded meta commands not working with kitty keyboard, Fish#303712
Fix non-hardcoded meta commands not working with kitty keyboard, Fish#303712anthonykim1 wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts integrated terminal keyboard event handling so VS Code can intercept Meta-modified keybindings before xterm.js consumes them when the kitty keyboard protocol is enabled (eg. fish enabling kitty protocol at the prompt).
Changes:
- Extend the xterm custom key event handler to skip xterm processing for Meta-modified keybindings (when
sendKeybindingsToShellis disabled). - Update inline comments explaining why Meta handling is needed under kitty keyboard protocol.
| // keyboard protocol, xterm.js encodes Meta-modified keys as CSI u sequences and | ||
| // consumes them via preventDefault. The (non-kitty) traditional xterm.js handler already skips | ||
| // Meta keys so they bubble up naturally, but the kitty handler does not. | ||
| if (!this._terminalConfigurationService.config.sendKeybindingsToShell && resolveResult.kind === ResultKind.KbFound && resolveResult.commandId && (event.metaKey || this._skipTerminalCommands.some(k => k === resolveResult.commandId))) { |
There was a problem hiding this comment.
The comment says keyboard events are skipped when they "use the Meta", but the implementation only skips Meta-modified keys when they also resolve to a VS Code command (ResultKind.KbFound with commandId). Please either adjust the wording to match the actual condition, or broaden the condition if the intent really is to skip all Meta-modified key events regardless of command resolution.
| if (!this._terminalConfigurationService.config.sendKeybindingsToShell && resolveResult.kind === ResultKind.KbFound && resolveResult.commandId && (event.metaKey || this._skipTerminalCommands.some(k => k === resolveResult.commandId))) { | |
| const shouldSkipForCommand = resolveResult.kind === ResultKind.KbFound && !!resolveResult.commandId && this._skipTerminalCommands.some(k => k === resolveResult.commandId); | |
| if (!this._terminalConfigurationService.config.sendKeybindingsToShell && (event.metaKey || shouldSkipForCommand)) { |
| // The metaKey check is needed because when a shell like fish enables the kitty | ||
| // keyboard protocol, xterm.js encodes Meta-modified keys as CSI u sequences and | ||
| // consumes them via preventDefault. The (non-kitty) traditional xterm.js handler already skips | ||
| // Meta keys so they bubble up naturally, but the kitty handler does not. | ||
| if (!this._terminalConfigurationService.config.sendKeybindingsToShell && resolveResult.kind === ResultKind.KbFound && resolveResult.commandId && (event.metaKey || this._skipTerminalCommands.some(k => k === resolveResult.commandId))) { |
There was a problem hiding this comment.
This change alters keyboard interception behavior for Meta-modified shortcuts when a keybinding resolves to a command (to avoid xterm.js consuming the event under kitty keyboard protocol). There doesn't appear to be unit coverage for TerminalInstance's custom key event handler logic; please add a test that simulates a Meta-modified keydown resolving to a command and verifies the handler returns false and calls preventDefault when sendKeybindingsToShell is disabled.
Resolves: #303118
I think when you press command shortcut in terminal, VS Code checks if the command is in hardcoded skip list:
vscode/src/vs/workbench/contrib/terminal/common/terminal.ts
Line 497 in a1254fd
If its like cmd+shift+p , vscode correctly intercepts it.
If not(meaning not in the above list) like cmd+w or zoom (cmd+), vscode lets xterm handle it.
I think with Kitty case with fish enabling KB protocol at prompt, xterm's kitty handler encodes these non-hardcoded command key as csi sequences -> sends to fish (which just ignores it), and then VS Code never see it.
Video after my PR:
https://github.com/user-attachments/assets/390c4b65-96fd-4027-bc4d-695285309dfb
This is really easy to test, just need to launch fish from mac and try things like cmd + (zoom in) or cmd+ shift+ w, with kitty keyboard protocol enabled.