Implement the Kitty Keyboard Protocol (#19817)#7
Implement the Kitty Keyboard Protocol (#19817)#7Dargon789 merged 1 commit intoDargon789:ci-main-clean-shellfrom
Conversation
This essentially rewrites `TerminalInput` from scratch. There's significant overlap between what kind of information the Kitty protocol needs from the OS and our existing code. The rewrite allows us to share large parts of the implementation. Closes #11509 ## Validation Steps Performed * `kitten show-key -m kitty` ✅ * US Intern. " + ' produces `\'` ✅ * Hebrew base keys produce Unicode ✅ * Hebrew AltGr combinations produce Unicode ✅ * French AltGr+Space produces U+00A0 ✅ * German AltGr+Decimals produce []{}... ✅
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Reviewer's GuideRewrites TerminalInput’s key handling to implement the Kitty Keyboard Protocol (KKP) on top of a new, shared encoding pipeline, wires new CSI u KKP control sequences through the VT parser/dispatch, adds settings and toggles in Terminal/ConPTY, and centralizes TAEF data-source helpers, with comprehensive unit tests verifying KKP behavior. Updated class diagram for TerminalInput and Kitty keyboard protocolclassDiagram
class TerminalInput {
+TerminalInput()
+UseAlternateScreenBuffer() void
+UseMainScreenBuffer() void
+SetInputMode(Mode mode, bool enabled) void
+GetInputMode(Mode mode) bool
+ResetInputModes() void
+ForceDisableWin32InputMode(bool win32InputMode) void
+ForceDisableKittyKeyboardProtocol(bool disable) void
+SetKittyKeyboardProtocol(uint8_t flags, KittyKeyboardProtocolMode mode) void
+GetKittyFlags() uint8_t
+PushKittyFlags(uint8_t flags) void
+PopKittyFlags(size_t count) void
+ResetKittyKeyboardProtocols() void
+HandleKey(INPUT_RECORD event) OutputType
+HandleFocus(bool focused) OutputType
+_makeWin32Output(KEY_EVENT_RECORD key) OutputType
-_encodeKitty(KeyboardHelper kbd, EncodingHelper enc, SanitizedKeyEvent key) bool
-_encodeRegular(EncodingHelper enc, SanitizedKeyEvent key) void
-_formatEncodingHelper(EncodingHelper enc, wstring seq) bool
-_formatFallback(KeyboardHelper kbd, EncodingHelper enc, SanitizedKeyEvent key, wstring seq) void
-_stringPushCodepoint(wstring str, uint32_t cp) void
-_codepointToLower(uint32_t cp) uint32_t
-_trackControlKeyState(KEY_EVENT_RECORD key) DWORD
-_initKeyboardMap() void
-_leadingSurrogate wchar_t
-_lastVirtualKeyCode optional~WORD~
-_lastControlKeyState DWORD
-_lastLeftCtrlTime uint64_t
-_lastRightAltTime uint64_t
-_inputMode enumset~Mode~
-_forceDisableWin32InputMode bool
-_inAlternateBuffer bool
-KittyStackMaxSize size_t
-_forceDisableKittyKeyboardProtocol bool
-_kittyFlags uint8_t
-_kittyMainStack vector~uint8_t~
-_kittyAltStack vector~uint8_t~
-_csi wstring_view
-_ss3 wstring_view
-_focusInSequence wstring_view
-_focusOutSequence wstring_view
}
class KittyKeyboardProtocolFlags {
<<struct>>
+None uint8_t
+DisambiguateEscapeCodes uint8_t
+ReportEventTypes uint8_t
+ReportAlternateKeys uint8_t
+ReportAllKeysAsEscapeCodes uint8_t
+ReportAssociatedText uint8_t
+All uint8_t
}
class KittyKeyboardProtocolMode {
<<enum>>
Replace
Set
Reset
}
class SanitizedKeyEvent {
<<struct>>
+virtualKey uint16_t
+scanCode uint16_t
+codepoint uint32_t
+controlKeyState uint32_t
+leftCtrlIsReallyPressed bool
+keyDown bool
+keyRepeat bool
+anyAltPressed() bool
+bothAltPressed() bool
+rightAltPressed() bool
+bothCtrlPressed() bool
+altGrPressed() bool
}
class KeyboardHelper {
<<struct>>
+KeyboardHelper()
+getUnmodifiedKeyboardKey(SanitizedKeyEvent key) uint32_t
+getKittyBaseKey(SanitizedKeyEvent key) uint32_t
+getKittyShiftedKey(SanitizedKeyEvent key) uint32_t
+getKittyUSBaseKey(SanitizedKeyEvent key) uint32_t
-getKeyboardKey(UINT vkey, DWORD controlKeyState, HKL hkl) uint32_t
-init() void
-initSlow() void
-_initialized bool
-_keyboardLayout HKL
-_keyboardState uint8_t[256]
}
class EncodingHelper {
<<struct>>
+EncodingHelper()
+shiftPressed() bool
+altPressed() bool
+ctrlPressed() bool
+csiUnicodeKeyCode uint32_t
+csiAltKeyCodeShifted uint32_t
+csiAltKeyCodeBase uint32_t
+csiModifier uint32_t
+csiEventType uint32_t
+csiTextAsCodepoint uint32_t
+csiFinal wchar_t
+ss3Final wchar_t
+altPrefix bool
+plain wstring_view
}
class CodepointBuffer {
<<struct>>
+CodepointBuffer()
+CodepointBuffer(uint32_t cp)
+convertLowercase() void
+asSingleCodepoint() uint32_t
+buf wchar_t[4]
+len int
}
class ITermDispatch {
<<interface>>
+SetKittyKeyboardProtocol(VTParameter flags, VTParameter mode) void
+QueryKittyKeyboardProtocol() void
+PushKittyKeyboardProtocol(VTParameter flags) void
+PopKittyKeyboardProtocol(VTParameter count) void
}
class AdaptDispatch {
+SetKittyKeyboardProtocol(VTParameter flags, VTParameter mode) void
+QueryKittyKeyboardProtocol() void
+PushKittyKeyboardProtocol(VTParameter flags) void
+PopKittyKeyboardProtocol(VTParameter count) void
-_terminalInput TerminalInput
}
TerminalInput *-- KittyKeyboardProtocolFlags
TerminalInput *-- KittyKeyboardProtocolMode
TerminalInput *-- SanitizedKeyEvent
TerminalInput *-- KeyboardHelper
TerminalInput *-- EncodingHelper
TerminalInput *-- CodepointBuffer
AdaptDispatch ..|> ITermDispatch
AdaptDispatch o--> TerminalInput
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello @Dargon789, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the implementation of the Kitty Keyboard Protocol, significantly enhancing the terminal's ability to report detailed keyboard input. The core Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.Unrecognized words (2)cimg To accept these unrecognized words as correct, you could run the following commands... in a clone of the git@github.com:microsoft/terminal.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.25/apply.pl' |
perl - 'https://github.com/Dargon789/terminal/actions/runs/22083313803/attempts/1' &&
git commit -m 'Update check-spelling metadata'Pattern suggestions ✂️ (1)You could add these patterns to Alternatively, if a pattern suggestion doesn't make sense for this project, add a Errors, Warnings, and Notices ❌ (3)See the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. ✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later. If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new Kitty keyboard protocol and encoding logic makes
terminalInput.cppquite large and dense; consider moving Kitty-specific helpers (_encodeKitty,_getKittyFunctionalKeyCode,KeyboardHelper,EncodingHelper, etc.) into a separate translation unit or namespace to keepTerminalInputmore focused and maintainable. - In
EncodingHelper::EncodingHelper()you usememset(this, 0, sizeof(*this)); this is fragile since the struct contains non-trivial types (std::wstring_view) and may gain more over time—prefer using in-class member initializers or an explicit member-wise constructor instead of zeroing the whole object memory.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new Kitty keyboard protocol and encoding logic makes `terminalInput.cpp` quite large and dense; consider moving Kitty-specific helpers (`_encodeKitty`, `_getKittyFunctionalKeyCode`, `KeyboardHelper`, `EncodingHelper`, etc.) into a separate translation unit or namespace to keep `TerminalInput` more focused and maintainable.
- In `EncodingHelper::EncodingHelper()` you use `memset(this, 0, sizeof(*this))`; this is fragile since the struct contains non-trivial types (`std::wstring_view`) and may gain more over time—prefer using in-class member initializers or an explicit member-wise constructor instead of zeroing the whole object memory.
## Individual Comments
### Comment 1
<location> `src/terminal/input/terminalInput.cpp:680-689` </location>
<code_context>
+ static constexpr auto lut = [] {
</code_context>
<issue_to_address>
**issue (bug_risk):** Functional-key LUT truncates high values to uint8_t, producing incorrect Kitty PUA key codes.
`lut` is `std::array<uint8_t, 256>`, but many `SET(...)` values exceed 255 (e.g. 57358/0xE03E, 57376/0xE040). They’re truncated to a single byte, so `_getKittyFunctionalKeyCode` does:
```cpp
const auto v = til::at(lut, vkey & 0xff);
return v <= KittyKeyCodeLegacySentinel ? v : 0xE000 + v;
```
For entries where you intended to store a full PUA code (as the comments like `// CAPS_LOCK`, `// F13` suggest), this becomes `0xE000 + (code & 0xff)`, which shifts the actual code point (e.g. 0xE040 becomes `0xE000 + 0x40`). Either store full PUA values in a `uint16_t`/`uint32_t` array and return them directly, or store 0–255 offsets and keep the current `0xE000 + v` calculation. As written, many mappings are incorrect due to truncation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Kitty Keyboard Protocol by rewriting TerminalInput. The new implementation is well-structured and includes a comprehensive set of unit tests, which is excellent. The refactoring of the TAEF data source adapter is a good cleanup, but it appears to have introduced a couple of memory leaks that need to be addressed. Overall, this is a high-quality contribution that adds a significant feature.
34ccabc
into
Dargon789:ci-main-clean-shell
This essentially rewrites
TerminalInputfrom scratch. There's significant overlap between what kind of information the Kitty protocol needs from the OS and our existing code. The rewrite allows us to share large parts of the implementation.Closes microsoft#11509
Validation Steps Performed
kitten show-key -m kitty✅\'✅Summary of the Pull Request
References and Relevant Issues
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
PR Checklist
Summary by Sourcery
Add support for the Kitty Keyboard Protocol with configurable flags, refactor terminal keyboard input encoding, and wire protocol control through escape sequences and profile settings.
New Features:
Enhancements:
Tests: