Kitty keyboard protocol: "Disambiguate escape codes" #811

Manually merged
dnkl merged 26 commits from kitty-kbd into master 2 months ago
dnkl commented 2 months ago
Owner

This is the first iteration of the first level of Kitty's keyboard protocol: "disambiguate escape codes" (\E[>1u)

On my Swedish layout, foot now appears to emit the same plain-text, and CSIs for everything (not every key + modifiers tested...), including keypad keys, AltGr combinations etc.

One thing that wasn't obvious was AltGr combinations. In general, we always emit the base symbol, i.e. the unshifted symbol. For example, Alt+2 emits the same 'key' as Alt+Shift+2. However, AltGr combinations seemed to always emit the shifted symbol. I believe this has to do with the modifier encoding: AltGr isn't included in the modifier set defined by the Kitty keyboard protocol. Thus, if we were to use the unshifted symbol, AltGr+Alt+2 would emit the same CSI as Alt+2.

Another interresting corner case is NumLock (and CapsLock, I presume). NumLock is a modifier, and does affect the CSIs we emit. But, it doesn't affect printable characters. That is, pressing A while NumLock is in effect still emits a plain 'A'. But, pressing Return, emits a CSI instead of a plain \r. This feels intuitive. But, I haven't found a way to generalize this.

From XKB's perspective, there's no difference between NumLock and e.g. Alt. Both are modifiers, and both change the key's shift level. NumLock is usually a "locked" key (i.e. pressing it once, "locks" it, pressing it again "unlocks" it). But all modifiers can behave like this (usually called "sticky keys").

For the time being, NumLock and CapsLock are explicitly filtered out from the set of effective modifiers when checking if the key is a printable.

Final oddity(?) is that e.g. NumLock has a designated CSI sequence, yet Kitty does not emit anything when it's toggled. I think the CSI is emitted in layouts where NumLock is not a modifier. This is what foot's implementation does, in any case.

Edit: these CSIs are for the "Report all keys as escape codes" mode.

Part of #319

@craigbarnes

This is the first iteration of the first level of Kitty's keyboard protocol: "disambiguate escape codes" (`\E[>1u`) On my Swedish layout, foot now appears to emit the same plain-text, and CSIs for everything (not **every** key + modifiers tested...), including keypad keys, <kbd>AltGr</kbd> combinations etc. One thing that wasn't obvious was <kbd>AltGr</kbd> combinations. In general, we always emit the base symbol, i.e. the _unshifted_ symbol. For example, <kbd>Alt</kbd>+<kbd>2</kbd> emits the same 'key' as <kbd>Alt</kbd>+<kbd>Shift</kbd>+<kbd>2</kbd>. However, <kbd>AltGr</kbd> combinations seemed to always emit the _shifted_ symbol. I believe this has to do with the modifier encoding: <kbd>AltGr</kbd> isn't included in the modifier set defined by the Kitty keyboard protocol. Thus, if we were to use the unshifted symbol, <kbd>AltGr</kbd>+<kbd>Alt</kbd>+<kbd>2</kbd> would emit the _same_ CSI as <kbd>Alt</kbd>+<kbd>2</kbd>. Another interresting corner case is <kbd>NumLock</kbd> (and <kbd>CapsLock</kbd>, I presume). <kbd>NumLock</kbd> _is_ a modifier, and _does_ affect the CSIs we emit. But, it doesn't affect printable characters. That is, pressing <kbd>A</kbd> while <kbd>NumLock</kbd> is in effect still emits a plain 'A'. But, pressing <kbd>Return</kbd>, emits a CSI instead of a plain `\r`. This feels intuitive. But, I haven't found a way to generalize this. From XKB's perspective, there's no difference between <kbd>NumLock</kbd> and e.g. <kbd>Alt</kbd>. Both are modifiers, and both change the key's shift level. <kbd>NumLock</kbd> is _usually_ a "locked" key (i.e. pressing it once, "locks" it, pressing it again "unlocks" it). But _all_ modifiers can behave like this (usually called "sticky keys"). For the time being, <kbd>NumLock</kbd> and <kbd>CapsLock</kbd> are explicitly filtered out from the set of effective modifiers _**when checking if the key is a printable**_. ~~Final oddity(?) is that e.g. <kbd>NumLock</kbd> has a designated CSI sequence, yet Kitty does not emit anything when it's toggled. I _think_ the CSI is emitted in layouts where <kbd>NumLock</kbd> is **not** a modifier. This is what foot's implementation does, in any case.~~ Edit: these CSIs are for the "Report all keys as escape codes" mode. Part of #319 @craigbarnes
dnkl added the
enhancement
label 2 months ago
dnkl added 14 commits 2 months ago
ea329453f0
input: refactor: new function: legacy_kbd_protocol()
b9089b125e
input: wip: add support for kitty kbd protocol “Disambiguate escape codes”
b105de12cf
input: kitty: use base symbol instead of lowering the symbol
5407cd5650
input: get_current_modifiers() no longer strips insignificant mods
8455ba2983
input: kitty: printables are emitted as text, even if Caps- or Num-Lock is in effect
b764875169
input: kitty: disable CSI for Caps- and Num-Lock
72de35c9ce
input: handle “invalid” XKB modifiers
09829f2377
input: rename ‘meta’ to ‘super’
dnkl added 3 commits 2 months ago
Poster
Owner

Final oddity(?) is that e.g. NumLock has a designated CSI sequence, yet Kitty does not emit anything when it's toggled.

When "Report all keys as escape codes" is enabled, modifier keys will generate CSIs.

> Final oddity(?) is that e.g. NumLock has a designated CSI sequence, yet Kitty does not emit anything when it's toggled. When "Report all keys as escape codes" is enabled, modifier keys will generate CSIs.
dnkl added 1 commit 2 months ago
craigbarnes reviewed 2 months ago
craigbarnes left a comment

@dnkl Thanks for working on this! I feel a bit guilty for saying I'd work it on for so long and then never getting around to it, but I think you've done a much better job of handling the important little details than I would've done.

I've looked over the code and tested it again briefly and it seems all good to me so far, aside from a few minor nits.

csi.c Outdated
grid->kitty_kbd.flags[idx] = flags;
grid->kitty_kbd.idx = idx;
xassert(grid->kitty_kbd.idx < ALEN(grid->kitty_kbd.flags));
Poster
Collaborator

This looks like a bounds check on an array that was already written to. It might be more logical to move it above line 1557 and change it to xassert(idx < ALEN(grid->kitty_kbd.flags));.

This looks like a bounds check on an array that was already written to. It might be more logical to move it above line 1557 and change it to `xassert(idx < ALEN(grid->kitty_kbd.flags));`.
dnkl commented 2 months ago
Poster
Owner

I just removed the assert. The idx calculation is simple enough that I think asserting isn't really necessary.

I just removed the assert. The `idx` calculation is simple enough that I think asserting isn't really necessary.
Poster
Collaborator

Yeah, agreed.

Yeah, agreed.
dnkl marked this conversation as resolved
input.c Outdated
[MOD_META | MOD_SHIFT | MOD_ALT | MOD_CTRL] = 16,
};
xassert(keymap_mods < sizeof(mod_param_map) / sizeof(mod_param_map[0]));
Poster
Collaborator

ALEN(mod_param_map) could be used here.

(not specific to this PR; but just mentioning here because I noticed it here)

`ALEN(mod_param_map)` could be used here. (not specific to this PR; but just mentioning here because I noticed it here)
dnkl commented 2 months ago
Poster
Owner

Fixed!

Fixed!
dnkl marked this conversation as resolved
input.c Outdated
int modify_param = mod_param_map[keymap_mods];
xassert(modify_param != 0);
char reply[1024];
Poster
Collaborator

This seems a little oversized for how it's used.

(also not specific to this PR; but mentioning anyway).

This seems a little oversized for how it's used. (also not specific to this PR; but mentioning anyway).
dnkl commented 2 months ago
Poster
Owner

Changed to 32.

Maximum length of a 32-bit integer-as-a-string is 10 chars. If I did the calculations right, that means the longest string we can generate is 27 chars + 1 NUL terminator.

Changed to 32. Maximum length of a 32-bit integer-as-a-string is 10 chars. If I did the calculations right, that means the longest string we can generate is 27 chars + 1 NUL terminator.
dnkl marked this conversation as resolved
input.c Outdated
* #1 is configured with \E[?1036, and is on by default
*
* If #1 has been disabled, we use #2, *if* it's a single
* byte we're emitting. Since this is an UTF-8 terminal,
Poster
Collaborator

"an UTF-8" -> "a UTF-8".

"an UTF-8" -> "a UTF-8".
dnkl commented 2 months ago
Poster
Owner

Fixed.

Fixed.
dnkl marked this conversation as resolved
dnkl added 3 commits 2 months ago
9d6d18684c
csi: kitty: remove unneeded xasserts
5ccf0719bf
input: legacy: reduce size of reply buffer
3d0e7ad01a
input: grammar: “an UTF-8” -> “a UTF-8”
dnkl added 1 commit 2 months ago
3fa7b8d101
input: legacy: use ALEN(mod_param_map)
dnkl changed title from WIP: kitty keyboard protocol: "Disambiguate escape codes" to Kitty keyboard protocol: "Disambiguate escape codes" 2 months ago
Poster
Owner

I removed the "WIP" status; there are no known missing features or differences compared with Kitty (for \E[>1u, that is).

I removed the "WIP" status; there are no known missing features or differences compared with Kitty (for `\E[>1u`, that is).
dnkl force-pushed kitty-kbd from 3fa7b8d101 to 5748d8ca06 2 months ago
dnkl force-pushed kitty-kbd from 5748d8ca06 to 302eeed667 2 months ago
dnkl added 1 commit 2 months ago
7e4ee4ba3f
term: rename: KITTY_KBD_MASK -> KITTY_KBD_SUPPORTED
dnkl force-pushed kitty-kbd from 7e4ee4ba3f to 9271886665 2 months ago
Poster
Owner

I updated the "flag" mask, so that we now only allow setting the features we support. This means that if an application tries to enable e.g \e[>3u, and then queries the current mode with \e[?u, we'll reply with \e[?1u (for the time being).

I updated the "flag" mask, so that we now only allow setting the features we support. This means that if an application tries to enable e.g `\e[>3u`, and then queries the current mode with `\e[?u`, we'll reply with `\e[?1u` (for the time being).
dnkl force-pushed kitty-kbd from 9271886665 to 901a87a5a0 2 months ago
Poster
Owner

Found one issue: pressing Control + KP_Multiply doesn't set the ctrl modifier. This is because XKB considers Control to be a "consumed" modifier in this case, and thus we ignore it when calculating the "effective" modifier bitmask.

Kitty does report ctrl in this case, so this is something that should be fixed.

I'm not sure how to proceed. I don't really want to ignore the consumed modifiers from XKB, since then we'd have to manually determine when to remove a modifier and when not to. I don't think we can ever get that right, in all possible layouts.

Found one issue: pressing <kbd>Control</kbd> + <kbd>KP_Multiply</kbd> doesn't set the `ctrl` modifier. This is because XKB considers <kbd>Control</kbd> to be a "consumed" modifier in this case, and thus we ignore it when calculating the "effective" modifier bitmask. Kitty _does_ report ctrl in this case, so this is something that should be fixed. I'm not sure how to proceed. I don't really want to ignore the consumed modifiers from XKB, since then we'd have to manually determine when to remove a modifier and when not to. I don't think we can ever get that right, in all possible layouts.
Poster
Owner

Maybe have a hardcoded list of non-consumable modifiers for all KP_* keys?

Maybe have a hardcoded list of non-consumable modifiers for all <kbd>KP_*</kbd> keys?
Poster
Owner

It appears to only affect the "operator" keys on the keypad (/, *, + and -). Their "consumed" mask appears to mask out all modifiers. Other keypad keys appear to work as expected.

It appears to only affect the "operator" keys on the keypad (`/`, `*`, `+` and `-`). Their "consumed" mask appears to mask out **all** modifiers. Other keypad keys appear to work as expected.
Poster
Owner

So, XKB has two different modes when getting the mask of consumed modifiers: XKB and GTK. Switching to GTK appears to produce the expected results for the keypad keys. Not sure if it has any other unforeseen effects... If we decide to use this, I think we should only do so in the kitty protocol (currently, internal bindings, the legacy kbd protocol and the kitty protocol all use the same function to retrieve the consumed modifiers).

Next problem: for some reason, I'm getting a weird xkb_keysym_t for Control+Alt+KP_Add. The result is "nothing", i.e. we don't emit anything. Other modifier combos work.

So, XKB has two different modes when getting the mask of consumed modifiers: `XKB` and `GTK`. Switching to `GTK` appears to produce the expected results for the keypad keys. Not sure if it has any other unforeseen effects... If we decide to use this, I think we should only do so in the kitty protocol (currently, internal bindings, the legacy kbd protocol and the kitty protocol all use the same function to retrieve the consumed modifiers). Next problem: for some reason, I'm getting a weird `xkb_keysym_t` for <kbd>Control</kbd>+<kbd>Alt</kbd>+<kbd>KP_Add</kbd>. The result is "nothing", i.e. we don't emit anything. Other modifier combos work.
dnkl added 2 commits 2 months ago
dnkl added 1 commit 2 months ago
dnkl force-pushed kitty-kbd from c3d2244d7a to 9e3b9f3a3b 2 months ago
dnkl force-pushed kitty-kbd from 9e3b9f3a3b to 7198f4e066 2 months ago
Poster
Owner

Next problem: for some reason, I'm getting a weird xkb_keysym_t for Control+Alt+KP_Add. The result is "nothing", i.e. we don't emit anything. Other modifier combos work.

The sym we're getting is 0x1008fe22, which is XKB_KEY_XF86Next_VMode (next video mode available). So, not really our problem I guess.

> Next problem: for some reason, I'm getting a weird xkb_keysym_t for Control+Alt+KP_Add. The result is "nothing", i.e. we don't emit anything. Other modifier combos work. The sym we're getting is `0x1008fe22`, which is `XKB_KEY_XF86Next_VMode` (_next video mode available_). So, not really our problem I guess.
dnkl force-pushed kitty-kbd from 7198f4e066 to 1f19dd6694 2 months ago
dnkl merged commit 4334259c70 into master manually 2 months ago
continuous-integration/woodpecker the build was successful
The pull request has been manually merged as 4334259c70.
Sign in to join this conversation.
Loading…
There is no content yet.