Conversation
This was referenced Jul 10, 2026
Merged
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (20)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughKitty keyboard protocol flags now propagate through terminal snapshots and protocol messages. Clients encode keys according to negotiated modes, while interactive parsing decodes CSI-u and modified legacy reports with safe fallbacks. Typed key bindings, passthrough actions, and tab-navigation fixtures were updated. ChangesKitty keyboard protocol
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Terminal
participant Server
participant Snapshot
participant Client
participant Program
Terminal->>Server: update kitty keyboard mode
Server->>Snapshot: publish keyboard_mode
Snapshot->>Client: deliver visible snapshot
Client->>Client: encode key using buffer mode
Client->>Program: send legacy or CSI-u bytes
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Pajn
force-pushed
the
csi-u
branch
2 times, most recently
from
July 11, 2026 07:07
9b4f478 to
6466f96
Compare
Pajn
force-pushed
the
smaller
branch
2 times, most recently
from
July 11, 2026 08:22
ce7ceef to
8121c78
Compare
Pajn
force-pushed
the
csi-u
branch
2 times, most recently
from
July 11, 2026 10:43
f4a313f to
fbb9350
Compare
Pajn
force-pushed
the
smaller
branch
2 times, most recently
from
July 13, 2026 09:27
4c1a2e9 to
cf89152
Compare
Pajn
force-pushed
the
csi-u
branch
2 times, most recently
from
July 13, 2026 10:21
116541f to
2da6314
Compare
Pajn
force-pushed
the
smaller
branch
2 times, most recently
from
July 13, 2026 12:08
17e79ca to
f1e8397
Compare
Pajn
force-pushed
the
csi-u
branch
2 times, most recently
from
July 14, 2026 07:52
76ebaf5 to
ba13544
Compare
Pajn
force-pushed
the
smaller
branch
2 times, most recently
from
July 15, 2026 07:26
d0a91e0 to
fee8dbe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 654-661: Update the keyboard escape-sequence parsing path around
the event-type check so key-release reports are represented as consumed/ignored
rather than returned as unrecognized. Ensure releases such as CSI 97;5:3u stop
without producing any input event or reaching the raw-byte fallback that creates
KeyEvent::Bytes. Preserve handling for press, repeat, and absent event types.
In `@crates/embers-client/src/controller.rs`:
- Around line 144-146: Update the KeyEvent::Key handling in map_key to pass the
focused buffer’s negotiated keyboard mode to encode_key instead of hardcoding 0.
Resolve or thread that mode through the surrounding input flow, preserving CSI-u
disambiguation for keys such as Ctrl+I.
In `@crates/embers-client/src/input/encoding.rs`:
- Around line 69-90: The encode_key function must route modes with either
KITTY_DISAMBIGUATE_ESC_CODES or KITTY_REPORT_ALL_KEYS_AS_ESC set through
encode_kitty; update the condition to use both mode predicates while preserving
legacy encoding when neither bit is set. In
crates/embers-client/src/input/encoding.rs lines 69-90, change encode_key only;
crates/embers-protocol/schema/embers.fbs lines 531-532 requires no direct change
because bit 4 is intentional.
- Around line 132-155: Update encode_legacy and ctrl_byte so legacy text keys
preserve shift and alt when combined with ctrl, including emitting alt before
the control byte for combinations such as Ctrl+Alt+x. Apply shift to character
encoding, and replace the blanket ASCII bitmask mapping with terminal-compatible
control mappings so Ctrl+Space and digit cases such as Ctrl+3 encode correctly.
Add coverage for Ctrl+Space, a digit control case, and mixed-modifier text keys
including Ctrl+Alt+x and Shift+a.
In `@crates/embers-client/src/input/keyparse.rs`:
- Around line 134-137: Update the key parsing logic around the modifier/key
split to preserve `-` as a valid base key: parse only recognized modifier
prefixes and treat the final hyphen as the base key, so bindings like `<C-->`
and `<C-S-->` do not create an empty modifier or return `InvalidModifier`. Add
regression coverage for both forms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de8940d2-f951-4b41-8c0c-449543fb92c5
📒 Files selected for processing (18)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/embers-client/src/configured_client.rs (1)
3326-3338: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute ordinary keys through the mode-aware encoder too.
In
KITTY_REPORT_ALL_KEYS_AS_ESC, unmatched characters and keys such as Tab still emit literal legacy bytes because these branches bypassencode_key. Route all supported key tokens through it; disambiguate-only mode already preserves their legacy encoding.Proposed direction
- KeyToken::Char(ch) => { - let mut encoded = [0; 4]; - bytes.extend_from_slice(ch.encode_utf8(&mut encoded).as_bytes()); - } - KeyToken::Space => bytes.push(b' '), - KeyToken::Tab => bytes.push(b'\t'), - KeyToken::Enter => bytes.push(b'\r'), - KeyToken::Backspace => bytes.push(0x7f), - KeyToken::Escape => bytes.push(0x1b), + KeyToken::Char(ch) => bytes.extend(crate::input::encode_key( + crate::input::KeyCode::Char(*ch), + crate::input::Modifiers::NONE, + mode, + )), + KeyToken::Space => bytes.extend(crate::input::encode_key( + crate::input::KeyCode::Char(' '), + crate::input::Modifiers::NONE, + mode, + )), + // Route Tab, Enter, Backspace, and Escape through encode_key likewise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/configured_client.rs` around lines 3326 - 3338, Update sequence_to_bytes to route every supported KeyToken, including ordinary characters and special keys such as Tab, through the mode-aware encode_key path when mode is KITTY_REPORT_ALL_KEYS_AS_ESC. Preserve legacy byte output in disambiguate-only mode and retain the existing handling for unsupported tokens or errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 667-724: Update the CSI key parsing flow around the key-code match
and modifier event-type handling so the final byte and key number are validated
first. Return None for unsupported keys before interpreting the event type; then
return Consumed only for event type 3, accept absent/1/2, and reject other event
types without consuming them.
In `@crates/embers-protocol/schema/embers.fbs`:
- Around line 531-532: Update the comment above keyboard_mode in the FlatBuffers
schema to document the correct Kitty keyboard protocol flags: bit 0/value 1 is
disambiguate, bit 3/value 8 is report-all, and bit 4/value 16 is
report-associated-text. Leave the keyboard_mode field unchanged.
---
Outside diff comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 3326-3338: Update sequence_to_bytes to route every supported
KeyToken, including ordinary characters and special keys such as Tab, through
the mode-aware encode_key path when mode is KITTY_REPORT_ALL_KEYS_AS_ESC.
Preserve legacy byte output in disambiguate-only mode and retain the existing
handling for unsupported tokens or errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc6e9c8f-d361-4e44-ae29-1161f29cc352
📒 Files selected for processing (18)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
Comment on lines
+531
to
+532
| // Appended: kitty keyboard protocol flags (bit 0 disambiguate, bit 4 report-all). | ||
| keyboard_mode:ubyte = 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the schema context around the commented lines.
sed -n '515,545p' crates/embers-protocol/schema/embers.fbs
# Find other mentions of kitty keyboard protocol flags or keyboard_mode.
rg -n "kitty keyboard protocol|keyboard_mode|report-all|report-associated-text|disambiguate" crates/embers-protocol -SRepository: Pajn/Embers
Length of output: 1634
🌐 Web query:
Kitty keyboard protocol report-all report-associated-text bit 3 bit 4 official documentation
💡 Result:
In the kitty keyboard protocol, progressive enhancement is managed using a bitmask of flags that applications send to the terminal to request specific reporting behaviors [1][2]. The flags you mentioned correspond to the following bit values within this mask: - Bit 3 (value 8): report_all_keys. This flag causes the terminal to report all key events as escape codes [1][3]. In the legacy protocol, keys that generate text (such as plain character keys) do not produce escape codes. Enabling this flag ensures that even these keys generate events, which is necessary for applications requiring key repeat or release notifications for all keys [1][4]. - Bit 4 (value 16): report_text. This flag is an enhancement to report_all_keys and causes key events that generate text to include the associated Unicode text (as codepoints) directly within the CSI u escape code [1][3]. Using this flag without report_all_keys is undefined [1]. In summary, when both bits are enabled (value 8 + 16 = 24), the terminal reports all key presses as escape codes and embeds the corresponding character text within those codes [1][5]. Sources: [6] https://sw.kovidgoyal.net/kitty/keyboard-protocol/ [1] https://github.com/kovidgoyal/kitty/blob/f13c8cd4/docs/keyboard-protocol.rst [3] https://deepwiki.com/kovidgoyal/kitty/5.3-keyboard-protocol-and-key-mapping [4] https://blessed.readthedocs.io/en/1.41/keyboard_kitty.html [5] https://ocaml.org/p/matrix/latest/doc/matrix.terminal/Terminal/index.html [2] https://terminfo.dev/extensions/kitty-keyboard-protocol
Citations:
- 1: https://github.com/kovidgoyal/kitty/blob/f13c8cd4/docs/keyboard-protocol.rst
- 2: https://terminfo.dev/extensions/kitty-keyboard-protocol
- 3: https://deepwiki.com/kovidgoyal/kitty/5.3-keyboard-protocol-and-key-mapping
- 4: https://blessed.readthedocs.io/en/1.41/keyboard_kitty.html
- 5: https://ocaml.org/p/matrix/latest/doc/matrix.terminal/Terminal/index.html
- 6: https://sw.kovidgoyal.net/kitty/keyboard-protocol/
Fix the Kitty flag docs
crates/embers-protocol/schema/embers.fbs: bit 3/value 8 is report-all, and bit 4/value 16 is report-associated-text; bit 0/value 1 is disambiguate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/embers-protocol/schema/embers.fbs` around lines 531 - 532, Update the
comment above keyboard_mode in the FlatBuffers schema to document the correct
Kitty keyboard protocol flags: bit 0/value 1 is disambiguate, bit 3/value 8 is
report-all, and bit 4/value 16 is report-associated-text. Leave the
keyboard_mode field unchanged.
Pajn
force-pushed
the
csi-u
branch
2 times, most recently
from
July 16, 2026 17:38
4acf6e6 to
ddd2fb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 643-652: Update decode_kitty_mods to return Option<Modifiers> and
reject masks containing unsupported kitty modifier bits for hyper, meta,
caps-lock, or num-lock. Return None for any unrepresentable mask so callers use
the raw-byte fallback, while returning Some with the existing shift, alt, ctrl,
and super_ mappings for supported masks.
- Around line 681-698: Update the final-byte matching logic in the interactive
key decoder so modified-legacy finals b'A' through b'S' are recognized only when
first equals 1; otherwise fall back to the existing raw-byte handling. Preserve
the current mappings for valid first == 1 sequences and the b'u' branch
behavior.
In `@crates/embers-client/src/configured_client.rs`:
- Around line 2304-2305: Update the search-mode key handling around the
KeyEvent::Key arm so unmodified CSI-u keys for ordinary characters, Tab, Enter,
Backspace, and Escape flow through the existing search branches instead of being
swallowed. Continue ignoring genuinely modified or unsupported keys, while
preserving the existing PageDown behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 47055bb0-b9fb-48c4-ac7a-69b17f3b3df3
📒 Files selected for processing (20)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/controller.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/integration.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
💤 Files with no reviewable changes (2)
- crates/embers-client/tests/integration.rs
- crates/embers-client/tests/controller.rs
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
crates/embers-client/src/input/encoding.rs (2)
77-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat report-all mode as disambiguated.
mode_disambiguates(0b1000)currently returns false, although report-all represents every key canonically and explicitly implies disambiguation. This public helper can therefore misroute report-all-only modes. (sw.kovidgoyal.net)Proposed fix
pub const fn mode_disambiguates(mode: u8) -> bool { - mode & KITTY_DISAMBIGUATE_ESC_CODES != 0 + mode & (KITTY_DISAMBIGUATE_ESC_CODES | KITTY_REPORT_ALL_KEYS_AS_ESC) != 0 }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/input/encoding.rs` around lines 77 - 84, Update the public mode_disambiguates function to return true when either KITTY_DISAMBIGUATE_ESC_CODES or KITTY_REPORT_ALL_KEYS_AS_ESC is enabled, so report-all-only mode is treated as disambiguated. Leave mode_reports_all_keys unchanged.
88-97: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSelect CSI-u based on whether the key combination has a legacy representation.
Under disambiguate-only mode, Shift-only text such as
<S-a>should remain text (A), but the nonempty modifier check emits CSI-u. Conversely, without negotiated flags, combinations such as Ctrl+Shift+A or Super+A have no legacy representation and must use CSI-u rather than dropping modifiers. The current Ctrl+Shift test consequently codifies the wrong result. (sw.kovidgoyal.net)Also applies to: 123-128, 139-166, 368-382
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/input/encoding.rs` around lines 88 - 97, Update encode_key to choose CSI-u based on whether the key/modifier combination has a valid legacy encoding, rather than solely on mode_disambiguates or mode_reports_all_keys. Preserve Shift-only text such as <S-a> as legacy output in disambiguate-only mode, while routing combinations without legacy representations (for example Ctrl+Shift+A and Super+A) through encode_kitty even when no flags are negotiated. Adjust the related tests covering these cases and the current Ctrl+Shift expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 647-648: Update decode_kitty_mods so modifier value 0 returns None
before subtracting the encoding offset; only values of at least 1 should convert
via value minus one into a Modifiers bitmask, preserving raw-byte fallback for
CSI 97;0u.
In `@crates/embers-client/src/configured_client.rs`:
- Around line 3299-3348: Apply normalize_text_key to unmodified CSI-u KeyEvents
before every input-routing path, including tokenization and the hints matching
path near the affected handlers. Ensure ordinary characters, Tab, Enter,
Backspace, and Escape become their legacy events while modified or non-text keys
remain unchanged. Add report-all input tests covering ordinary bindings,
Tab/Space bindings, and hints labels.
In `@crates/embers-client/src/input/encoding.rs`:
- Around line 168-171: Update encode_legacy’s KeyCode handling to consult the C0
modifier table before emitting default bytes for Enter, Tab, Backspace, and
Escape. Include Shift+Tab/backtab, Alt+Enter, Ctrl+Backspace, and Ctrl+Shift+Tab
mappings, while preserving unmodified legacy byte behavior.
---
Duplicate comments:
In `@crates/embers-client/src/input/encoding.rs`:
- Around line 77-84: Update the public mode_disambiguates function to return
true when either KITTY_DISAMBIGUATE_ESC_CODES or KITTY_REPORT_ALL_KEYS_AS_ESC is
enabled, so report-all-only mode is treated as disambiguated. Leave
mode_reports_all_keys unchanged.
- Around line 88-97: Update encode_key to choose CSI-u based on whether the
key/modifier combination has a valid legacy encoding, rather than solely on
mode_disambiguates or mode_reports_all_keys. Preserve Shift-only text such as
<S-a> as legacy output in disambiguate-only mode, while routing combinations
without legacy representations (for example Ctrl+Shift+A and Super+A) through
encode_kitty even when no flags are negotiated. Adjust the related tests
covering these cases and the current Ctrl+Shift expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69958558-6931-4f33-af51-5733fcd0ec61
📒 Files selected for processing (20)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/controller.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/integration.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
💤 Files with no reviewable changes (2)
- crates/embers-client/tests/integration.rs
- crates/embers-client/tests/controller.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 712-717: Update the modifier parsing in the event-report handling
match to reject extra colon-separated fields: after parsing the value and event
type from modifier.split(':'), ensure no further component remains and return
None when one does. Preserve valid reports with at most one event-type field and
the existing raw-byte fallback for malformed input.
In `@crates/embers-client/src/input/encoding.rs`:
- Around line 166-173: Update the mode-aware encoder around ctrl_byte and the
Ctrl-chord branch in crates/embers-client/src/input/encoding.rs:166-173 to
return an encoding error for non-ASCII Ctrl chords when Kitty mode is
unavailable, while retaining existing ASCII encoding. In
crates/embers-client/src/input/keyparse.rs:175-182, preserve the typed Ctrl
chord instead of rejecting it during parsing so validation is deferred to
encoding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33816cba-9d39-4729-94a8-35d7a602fceb
📒 Files selected for processing (20)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/controller.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/integration.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
💤 Files with no reviewable changes (2)
- crates/embers-client/tests/integration.rs
- crates/embers-client/tests/controller.rs
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 755-771: Remove the b'R' mapping in the CSI key decoder so
cursor-position reports such as CSI 1;2R fall back to raw bytes instead of
becoming F3. Preserve F3 decoding through the existing tilde-sequence match by
adding or updating the 13;mods~ mapping, and add regression tests for raw CSI
1;2R and Shift+F3 from CSI 13;2~.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce2f721b-1ec0-4e59-a82a-588b158352fa
📒 Files selected for processing (20)
crates/embers-cli/src/interactive.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/controller.rscrates/embers-client/src/input/encoding.rscrates/embers-client/src/input/keyparse.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/lib.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/controller.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/integration.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/support/mod.rscrates/embers-core/src/snapshot.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rs
💤 Files with no reviewable changes (2)
- crates/embers-client/tests/controller.rs
- crates/embers-client/tests/integration.rs
Foundation for extended-keys support, added additively so legacy encodings
and bindings are unchanged:
- input/encoding.rs: a single encode_key(code, mods, mode) used by both the
host controller and the scripted send-keys path. Emits legacy VT sequences
by default and kitty CSI-u sequences when the target's keyboard mode has
DISAMBIGUATE_ESC_CODES (honoring REPORT_ALL_KEYS_AS_ESC); KeyCode +
Modifiers model the logical key
- keyparse grammar: S- shift, multi-modifier (<C-S-Tab>), and function keys
<F1>-<F12> now parse into a new KeyToken::Key { code, mods }; single
C-/A- char keys still produce the legacy Ctrl/Alt tokens
- KeyEvent::Key alongside the legacy variants; key_event_to_token and
sequence_to_bytes handle it via the shared encoder
Tests: encoder table (legacy vs disambiguate vs report-all, modified arrows,
C-S-Tab, function keys) and keyparse grammar (<C-S-Tab>, <S-Left>, <F5>,
invalid combos).
- terminal backend enables alacritty's kitty_keyboard, so inner apps can push/pop/query disambiguation flags; the CSI ? u query reply rides Event::PtyWrite (already forwarded) - the active kitty flags are read into a compact keyboard_mode bitfield (bit 0 disambiguate, bit 4 report-all-keys) on TerminalModes / BackendMetadata and flow through the visible snapshot to the protocol (VisibleSnapshotResponse.keyboard_mode) and on to the client, mirroring bracketed_paste; the client encoder is ready to consume it Tests: backend push/pop of the kitty mode; protocol round-trip of a non-zero keyboard_mode.
- interactive client pushes the kitty keyboard protocol (disambiguate-only, \x1b[>1u) on enter and pops it (\x1b[<u) on exit; unsupported terminals ignore both harmlessly - parse_extended_key decodes CSI-u (code;mods u) and modified-legacy (CSI 1;mods A-D/H/F, CSI n;mods ~, F-key letters) reports into KeyEvent::Key, ignoring event-type sub-parameters; unknown sequences still fall back to KeyEvent::Bytes passthrough - passthrough coherence: unmatched keys re-encode through the shared encoder using the focused buffer's negotiated keyboard mode (buffer_keyboard_mode), so csi-u in yields csi-u out when the inner app wants it and legacy translation when it doesn't; the SendKeys action and alternate-screen binding passthrough use it too - fixture config binds <C-Tab>/<C-S-Tab> to next/prev tab Tests: parser units (CSI-u, C-S-Tab, modified arrows, modified F-key, fallback to bytes).
Controller::map_key was a legacy input mapper superseded by the ConfiguredClient input path (bindings, modes, and encode_key with the buffer's negotiated keyboard mode). It was only exercised by its own tests, never in the live flow, and hardcoded keyboard mode 0. Remove the Controller struct, map_key, and its private helpers (ctrl_byte, alt_bytes_request, input_request), drop tests/controller.rs and the crate export. The shared KeyEvent/MouseEvent input types in the same module are kept — they remain in live use.
The compact keyboard_mode byte put report-all-keys at bit 4, which is the kitty protocol's report-associated-text position — an internally consistent but confusing reuse of a spec bit for a different flag. Move report-all-keys to bit 3 (its kitty-spec position) across the producer (terminal_backend::kitty_keyboard_mode), the consumer constant (KITTY_REPORT_ALL_KEYS_AS_ESC), and the field docs (snapshot.rs, the FlatBuffers schema). Bit 0 = disambiguate and bit 3 = report-all now match the protocol's own numbering, so the byte never reuses a spec bit position for a different meaning.
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.
Stack
Summary by CodeRabbit