feat: Complete legacy keyboard protocol for extra keys

[gdamore]

Summary by CodeRabbit

• Tests

  • Expanded keyboard event tests to cover SS3 and CSI F-keys, modifier combinations, and many control/key combinations (return, tab, backspace, ctrl/alt variants).

• Refactor

  • Reworked keyboard handling to use a data-driven mapping and dynamic sequence generation, improve Alt modifier emission, and unify legacy and CSI F-key behavior for more consistent output.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors terminal keyboard event handling by introducing a centralized legacyKeys lookup and changes KeyEvent emission logic (modifier-aware SS3/CSI generation, Alt prefixing, expanded fallbacks). Adds extensive tests covering SS3 (F1–F4), CSI (F5–F9), and miscellaneous control key sequences and modifiers.

Changes

Cohort / File(s)	Summary
Test Expansion mock/term_test.go	Adds many new scenarios to TestKbdEventLegacy verifying SS3-based F1–F4 and CSI-based F5–F9 with modifiers, plus miscellaneous control keys (return, tab, ctrl-tab, ctrl-m, ctrl-l, backspace variants, ctrl-space, space) asserting exact byte/escape outputs.
Key Handling Refactor vt/emulate.go	Introduces legacyKeys data-driven map and replaces ad-hoc switch-case handling with modifier-aware lookup and dynamic sequence construction (SS3→CSI translation when needed), explicit Alt (ESC) prefixing, expanded Ctrl/printable fallbacks, and removal of the previous single-path per-key handling.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat(emulator): Start of keyboard processing (very preliminary) #936 — Refactors keyboard handling; strong overlap with this PR’s key-sequence mapping and emulator test changes.
• fix: add legacy SS3 modifier support #894 — Expands SS3 modifier handling for function keys; closely related to the SS3/CSI modifier logic adjusted here.

Poem

🐰 I tapped the keys and watched them fly,
ESC and CSI danced by and by,
F-keys switched their tidy tune,
Modifiers hummed beneath the moon,
Tests hop, green lights—what a joy to spy! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Complete legacy keyboard protocol for extra keys' accurately and clearly summarizes the main change: expanding keyboard event handling to support additional keys in the legacy terminal protocol.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a76cafe and 9034b90.

📒 Files selected for processing (2)

• mock/term_test.go
• vt/emulate.go

🔇 Additional comments (5)

mock/term_test.go (3)

436-457: LGTM! Good test coverage for SS3-based F-keys with modifiers.

The tests correctly validate:

• Unmodified F1 produces SS3 P (\x1bOP)
• Key release (Down: false) produces no output
• Modified F-keys convert SS3 to CSI format with modifier parameter
• Alt modifier correctly prefixes with ESC

---

459-479: LGTM! CSI-based F-key tests are correct.

The tests properly validate CSI sequence format with modifiers (e.g., \x1b[17;2~ for Shift+F6) and ESC prefix for Alt combinations.

---

481-508: LGTM! Comprehensive miscellaneous key tests.

Good coverage of edge cases including:

• Control key combinations (Ctrl+m → \r, Ctrl+l → \x0c)
• Backspace variants with modifiers
• Correct rejection of unsupported modifiers (Hyper, Meta producing no output)
• Alt prefix behavior for Alt+a and Alt+Ctrl+j

vt/emulate.go (2)

983-1000: LGTM! Clean modifier encoding logic.

The SS3-to-CSI conversion and CSI modifier insertion are correctly implemented following the standard terminal escape sequence conventions.

---

962-1006: LGTM! Well-structured key lookup with modifier handling.

The data-driven approach using legacyKeys map improves maintainability. The TODO for keypad mode (line 967) is a reasonable placeholder for future implementation.

Minor note: The P field in the struct is declared but not yet used; consider tracking this as a follow-up task if keypad mode support is planned.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.00%. Comparing base (561bb68) to head (9034b90).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
vt/emulate.go	95.55%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
+ Coverage   60.62%   61.00%   +0.38%     
==========================================
  Files          37       37              
  Lines        3256     3293      +37     
==========================================
+ Hits         1974     2009      +35     
- Misses       1124     1125       +1     
- Partials      158      159       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

mock/term_test.go (1)

481-502: Good coverage for special keys.

The test correctly validates the legacyKeys mappings for Tab, Backspace, Space, and Return with various modifiers.

Consider adding a test case for Alt+Ctrl+letter (e.g., Alt+Ctrl+a) to verify the fallback path handles the Alt prefix correctly — this would also catch the dead code issue mentioned in the emulate.go review.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 561bb68 and a76cafe.

📒 Files selected for processing (2)

• mock/term_test.go
• vt/emulate.go

🧰 Additional context used

🧬 Code graph analysis (2)

mock/term_test.go (1)

vt/kbd.go (11)

• KbdEvent (17-24)
• KcF1 (193-193)
• ModShift (30-30)
• KcF2 (194-194)
• ModCtrl (31-31)
• KcF3 (195-195)
• ModAlt (32-32)
• KcReturn (62-62)
• KcTab (61-61)
• KcBackspace (60-60)
• KcSpace (64-64)

vt/emulate.go (2)

vt/kbd.go (36)

• KeyCode (56-56)
• KcF1 (193-193)
• KcF2 (194-194)
• KcF3 (195-195)
• KcF4 (196-196)
• KcF5 (197-197)
• KcF6 (198-198)
• KcF7 (199-199)
• KcF8 (200-200)
• KcF9 (201-201)
• KcF10 (202-202)
• KcF11 (203-203)
• KcF12 (204-204)
• KcUp (80-80)
• KcDown (81-81)
• KcRight (83-83)
• KcLeft (82-82)
• KcHome (91-91)
• KcEnd (92-92)
• KcPgUp (89-89)
• KcPgDn (90-90)
• KcDel (94-94)
• KcIns (93-93)
• KcMenu (70-70)
• KcTab (61-61)
• KcBackspace (60-60)
• KcDelete (65-65)
• KcSpace (64-64)
• KcReturn (62-62)
• KcEsc (63-63)
• KbdEvent (17-24)
• ModMeta (33-33)
• ModShift (30-30)
• ModCtrl (31-31)
• ModNone (29-29)
• ModAlt (32-32)

input.go (1)

• P (887-887)

🔇 Additional comments (5)

vt/emulate.go (3)

916-952: Well-structured legacy key mapping table.

The centralized legacyKeys map is a good approach for maintainability. A few observations:

1. The P field for keypad mode is defined for arrow/navigation keys but the TODO at line 967 indicates it's not yet implemented.

2. Consider documenting the modifier encoding convention (e.g., that CSI sequences use mod+1 where shift=1, ctrl=4).

---

983-1006: Modifier sequence construction looks correct.

The logic properly handles:

• SS3-to-CSI conversion for modified F1-F4 keys
• Modifier encoding using xterm conventions (shift=+1, ctrl=+4)
• ESC prefix for Alt key

One minor note: Line 999 assumes v.K has length ≥ 1. This is safe given the current map entries, but you might want defensive checks if the map could be extended externally.

---

1018-1024: LGTM!

The fallback for printable ASCII keys correctly handles the Alt modifier using bitwise check, unlike the control-key fallback above.

mock/term_test.go (2)

436-457: Good test coverage for SS3-based F-keys.

The tests correctly validate:

• Unmodified SS3 sequences
• SS3-to-CSI conversion with modifiers
• Alt prefix handling with combined modifiers
• Key release events (Down: false) producing no output

---

459-479: LGTM!

CSI-based F-key tests correctly validate the modifier insertion format (;{mod} before the final ~).