On key-down, the shift modifier is lost in key input encoding (between raw_key_event_impl and key_event_impl)

[cstrahan]

What Operating System(s) are you seeing this problem on?

macOS

Which Wayland compositor or X11 Window manager(s) are you using?

N/A

WezTerm version

20240212-074107-22f9f8d2

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

On the initial key-down event, the SHIFT modifier will be lost by the time key_event_impl is called. This happens to be fine for the regular encoding (all we care about is the key code char), but for kitty keyboard protocol we need to know the modifier status to get the encoding right.

See this scenario, where I hold SHIFT and tap a:

21:13:32.943  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
21:13:32.944  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None }
21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;1;65u"
21:13:32.951  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:13:33.044  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
21:13:33.045  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
21:13:33.045  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;2:3u"

In raw_key_event_impl, we get the expected RawKeyEvent for the initial key-down input; from above:

21:13:32.943  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }

But then in key_event_impl (which seems to be where the encoding actually occurs) we see that the KeyEvent does not have the respective RawKeyEvent, and the SHIFT modifier is missing on the KeyEvent itself:

21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None }

This results in an improperly encoded input under the kitty keyboard protocol:

21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;1;65u"

Instead of "\u{1b}[97:65;1;65u", this should be "\u{1b}[97:65;2;65u", but because we don't have the SHIFT modifier, we end up encoding the empty set of modifiers.

Interestingly, the key-up case works as expected; the RawKeyEvent is preserved, as well as the modifiers directly within the KeyEvent, and we get: "\u{1b}[97:65;2:3u"

Now, instead of SHIFT, compare with CTRL+a:

21:40:26.939  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
21:40:26.939  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: true, raw: Some(RawKeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }) }
21:40:26.939  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97;5;97u"
21:40:27.086  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
21:40:27.086  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('a'), modifiers: CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
21:40:27.086  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97;5:3u"

Note that the CTRL modifier has persisted, as well as the RawKeyEvent.

To Reproduce

• Run WEZTERM_LOG=wezterm_input_types=debug,info wezterm start from another terminal.
• In wezterm, hold SHIFT and tap a.
• You'll see the aforementioned log lines.

Configuration

local config = {}
config.debug_key_events = true
config.enable_kitty_keyboard = true

return config

Expected Behavior

The modifiers should be preserved throughout the input event handling.

Logs

21:13:32.943  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
21:13:32.944  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None }
21:13:32.951  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;1;65u"
21:13:32.951  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:13:33.044  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
21:13:33.045  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
21:13:33.045  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;2:3u"

Anything else?

No response

[cstrahan]

This has something to do with the code path that handles IME, at least on the macOS backend; if I set config.use_ime = false (default is true) then the raw: RawKeyCode is preserved:

21:52:23.577  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
21:52:23.579  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }) }
21:52:23.579  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;2;65u"
21:52:23.690  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
21:52:23.691  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
21:52:23.691  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;2:3u"

Config in full:

local config = {}
config.debug_key_events = true
config.enable_kitty_keyboard = true
config.use_ime = false

return config

[cstrahan]

With IME enabled again, here are some logs doing the SHIFT+a test:

21:59:11.456  TRACE  window                                > AdviseModifiersLedStatus(SHIFT, (empty))
21:59:11.457  TRACE  window                                > Notification(Any { .. })
21:59:11.458  TRACE  window                                > NeedRepaint
21:59:11.463  TRACE  window                                > Notification(Any { .. })
21:59:11.463  TRACE  window                                > Notification(Any { .. })
21:59:11.667  DEBUG  window::os::macos::window             > key_common: chars=`A` unmod=`A` modifiers=`SHIFT` virtual_key=0 key_is_down:true
21:59:11.668  TRACE  window                                > RawKeyEvent(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) })
21:59:11.668  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
21:59:11.668  TRACE  window::os::macos::window             > insert_text_replacement_range A NSRange { location: 9223372036854775807, length: 0 }
21:59:11.668  TRACE  window                                > AdviseDeadKeyStatus(None)
21:59:11.668  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:59:11.669  TRACE  window                                > KeyEvent(KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None })
21:59:11.669  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None }
21:59:11.669  INFO   wezterm_gui::termwindow::keyevent     > send to pane DOWN key=Char('A') mods=NONE
21:59:11.669  INFO   wezterm_term::terminalstate::keyboard > key_down: sending "A", Char('A') NONE
21:59:11.669  TRACE  window::os::macos::window             > IME state: Acted, last_event: Some(KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: true, raw: None })
21:59:11.669  TRACE  window                                > AdviseDeadKeyStatus(None)
21:59:11.669  INFO   wezterm_gui::termwindow               > DeadKeyStatus now: None
21:59:11.670  TRACE  window                                > Notification(Any { .. })
21:59:11.671  TRACE  window                                > Notification(Any { .. })
21:59:11.671  TRACE  window                                > NeedRepaint
21:59:11.675  TRACE  window                                > Notification(Any { .. })
21:59:11.675  TRACE  window                                > Notification(Any { .. })
21:59:11.675  TRACE  window                                > Notification(Any { .. })
21:59:11.675  TRACE  window                                > Notification(Any { .. })
21:59:11.678  TRACE  window                                > Notification(Any { .. })
21:59:11.685  TRACE  window                                > Notification(Any { .. })
21:59:11.696  TRACE  window                                > NeedRepaint
21:59:11.769  DEBUG  window::os::macos::window             > key_common: chars=`A` unmod=`A` modifiers=`SHIFT` virtual_key=0 key_is_down:false
21:59:11.769  TRACE  window                                > RawKeyEvent(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) })
21:59:11.769  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
21:59:11.769  DEBUG  window::os::macos::window             > key_common KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) } (chars="A" unmod="A" modifiers=SHIFT)
21:59:11.769  TRACE  window                                > KeyEvent(KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) })
21:59:11.770  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: NONE, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
21:59:11.770  INFO   wezterm_gui::termwindow::keyevent     > send to pane UP key=Char('A') mods=NONE
21:59:11.770  TRACE  window                                > NeedRepaint
21:59:11.855  TRACE  window                                > AdviseModifiersLedStatus(NONE, (empty))

[cstrahan]

I'm pretty sure this is where things are going wrong, in insert_text_replacement_range:

https://github.com/wez/wezterm/blob/889f8a9cd71a2b3552f28f6d1864aa3cd9461fdf/window/src/os/macos/window.rs#L1873-L1880

The kitty input encoder expects to fish out the modifiers from raw (the RawKeyEvent), but we're setting it to None here when the IME completes and calls us back. I'm not really sure what the proper solution is here.

Do we squirrel away the last RawKeyEvent from within key_common, like we do with key_is_down? Or just the modifiers, and then synthesize a suitable RawKeyEvent within insert_text_replacement_range?

[cstrahan]

For comparison, here's SHIFT+CTRL+a with IME enabled, which encodes correctly (preserves modifiers):

22:55:09.236  TRACE  window::os::macos::window             > perform_key_equivalent: chars=`\u{1}` modifiers=`SHIFT | CTRL`
22:55:09.237  DEBUG  window::os::macos::window             > key_common: chars=`\u{1}` unmod=`A` modifiers=`SHIFT | CTRL` virtual_key=0 key_is_down:true
22:55:09.237  TRACE  window                                > RawKeyEvent(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) })
22:55:09.238  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }
22:55:09.238  TRACE  window::os::macos::window             > do_command_by_selector "moveToBeginningOfParagraphAndModifySelection:"
22:55:09.238  TRACE  window::os::macos::window             > IME state: Continue, last_event: None
22:55:09.238  DEBUG  window::os::macos::window             > key_common KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: true, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }) } (chars="\u{1}" unmod="A" modifiers=SHIFT | CTRL)
22:55:09.238  TRACE  window                                > KeyEvent(KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: true, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }) })
22:55:09.238  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: true, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: true, handled: Handled(false) }) }
22:55:09.238  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;6;65u"
22:55:09.240  TRACE  window                                > NeedRepaint
22:55:09.243  TRACE  window                                > Notification(Any { .. })
22:55:09.259  TRACE  window                                > NeedRepaint
22:55:09.399  DEBUG  window::os::macos::window             > key_common: chars=`\u{1}` unmod=`A` modifiers=`SHIFT | CTRL` virtual_key=0 key_is_down:false
22:55:09.400  TRACE  window                                > RawKeyEvent(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) })
22:55:09.400  INFO   wezterm_gui::termwindow::keyevent     > key_event RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }
22:55:09.400  DEBUG  window::os::macos::window             > key_common KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) } (chars="\u{1}" unmod="A" modifiers=SHIFT | CTRL)
22:55:09.400  TRACE  window                                > KeyEvent(KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) })
22:55:09.400  INFO   wezterm_gui::termwindow::keyevent     > key_event KeyEvent { key: Char('A'), modifiers: CTRL, leds: (empty), repeat_count: 1, key_is_down: false, raw: Some(RawKeyEvent { key: Char('A'), modifiers: SHIFT | CTRL, leds: (empty), phys_code: Some(A), raw_code: 0, repeat_count: 1, key_is_down: false, handled: Handled(false) }) }
22:55:09.400  INFO   wezterm_gui::termwindow::keyevent     > kitty: Encoded input as "\u{1b}[97:65;6:3u"

[cstrahan]

https://github.com/wez/wezterm/blob/889f8a9cd71a2b3552f28f6d1864aa3cd9461fdf/window/src/os/macos/window.rs#L2723-L2738

I suppose flags_changed could persist the current modifiers, which could then be read in insert_text_replacement_range to create a suitable RawKeyEvent. That would also lay the groundwork for emitting key-{down,up} events for the modifiers themselves (since, unless I'm mistaken, the macOS libs doesn't provide a way to plumb those directly through keyDown:/keyUp:).

@wez do you have any opinions on any of the above? I'd be happy to contribute patches, if I knew what angle to tackle things from.

[wez]

I'll be a bit distracted for the next couple of weeks so I don't have time right now to look at this in detail and make a suggestion, but I am definitely willing to accept a PR that makes this better if you have the bandwidth to look at it!

[cstrahan]

@wez Alrighty -- I'll see if I can cook something up. Disabling IME is a decent workaround for now (I don't take advantage of IME at the moment anyway), though I'd love for everyone be able to have their cake and eat it too.

And thanks again for wezterm and all of your other open source work!

[sirburpalot]

macOS user here.

WezTerm does not appear to report key down / key up events for any modifiers (i.e. Shift, Ctrl or Alt) when using the Kitty keyboard mode. This can be checked using kitten show_key -m kitty (kitten is a command-line utility that comes bundled with Kitty). Setting config.use_ime to false does not solve the problem.

[cstrahan]

Setting config.use_ime to false does not solve the problem.

Correct. I think we'd have to do something like I describe in https://github.com/wez/wezterm/issues/5193#issuecomment-2008648763 (track active modifiers in flags_changed, and emit events when the state changes) in order to fix that.