Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

key input issue - clean_sym vs composed_sym ? #1980

Closed
bew opened this issue Sep 18, 2019 · 32 comments
Closed

key input issue - clean_sym vs composed_sym ? #1980

bew opened this issue Sep 18, 2019 · 32 comments

Comments

@bew
Copy link
Contributor

bew commented Sep 18, 2019

Hello,

I'm having an issue with the keybind Alt+/ not doing anything in terminal apps (cat, zsh, nvim, ..)

I'm using the french keyboard layout, which has the slash and colon symbols on the same key (to get the slash I do Shift+colon).

The actual keys I have to press are Alt Shift colon (useful for the debug output below).

Here is the relevant debug output:

Press scancode: 0x40 clean_sym: Alt_L composed_sym: Alt_L mods: none glfw_key: 342 (LEFT ALT) xkb_key: 65513 (Alt_L)
on_key_input: glfw key: 342 native_code: 0xffe9 action: PRESS mods: 0x0 text: '' state: 0 sent key to child

Press scancode: 0x32 clean_sym: Shift_L composed_sym: Shift_L mods: alt glfw_key: 340 (LEFT SHIFT) xkb_key: 65505 (Shift_L)
on_key_input: glfw key: 340 native_code: 0xffe1 action: PRESS mods: 0x4 text: '' state: 0 sent key to child

Press scancode: 0x3c clean_sym: colon composed_sym: slash mods: alt+shift glfw_fallback_key: 46 (PERIOD) xkb_key: 46 (period)
on_key_input: glfw key: 46 native_code: 0x2e action: PRESS mods: 0x5 text: '' state: 0 sent key to child

Release scancode: 0x3c clean_sym: colon mods: alt+shift glfw_fallback_key: 46 (PERIOD) xkb_key: 46 (period)
on_key_input: glfw key: 46 native_code: 0x2e action: RELEASE mods: 0x5 text: '' state: 0 ignoring as keyboard mode does not allow release events

Release scancode: 0x32 clean_sym: Shift_L mods: alt+shift glfw_key: 340 (LEFT SHIFT) xkb_key: 65505 (Shift_L)
on_key_input: glfw key: 340 native_code: 0xffe1 action: RELEASE mods: 0x5 text: '' state: 0 ignoring as keyboard mode does not allow release events

Release scancode: 0x40 clean_sym: Alt_L mods: alt glfw_key: 342 (LEFT ALT) xkb_key: 65513 (Alt_L)
on_key_input: glfw key: 342 native_code: 0xffe9 action: RELEASE mods: 0x4 text: '' state: 0 ignoring as keyboard mode does not allow release events

The most interesting lines seems to be:

Press scancode: 0x3c clean_sym: colon composed_sym: slash mods: alt+shift glfw_fallback_key: 46 (PERIOD) xkb_key: 46 (period)
on_key_input: glfw key: 46 native_code: 0x2e action: PRESS mods: 0x5 text: '' state: 0 sent key to child

We can see clean_sym: colon composed_sym: slash, but I'm not sure what is actually sent for the on_key_input since the text: '' is empty..

Tested on version 0.14.4 and git version.
On Archlinux, with X11.

@bew
Copy link
Contributor Author

bew commented Sep 18, 2019

Same issue with another keybind: Alt+#

The numbersign (#) is on the same key as quotedbl (") and the number 3:

  • quotedbl is the direct symbol
  • 3 is obtained with shift
  • numbersign is obtained with AltGr (a special key on french keyboard, Alt_R in practice)

The relevant debug log for the key press:

Press scancode: 0xc clean_sym: quotedbl composed_sym: numbersign active_unknown_mods: Mod5 consumed_unknown_mods: Mod5 mods: alt glfw_fallback_key: 51 (3) xkb_key: 51 (3)
on_key_input: glfw key: 51 native_code: 0x33 action: PRESS mods: 0x4 text: '' state: 0 sent key to child

We have: clean_sym: quotedbl composed_sym: numbersign.

Btw, I'm not sure what active_unknown_mods: Mod5 consumed_unknown_mods: Mod5 is about, my mod5 is configured as mod5 ISO_Level3_Shift (0x5c), Mode_switch (0xcb) (from xmodmap), no idea what those 2 keys are.. But I don't think it's useful here.


It looks like the solution would be to use the composed_sym field of the press event, since it seems to be always valid when present.

@Luflosi
Copy link
Contributor

Luflosi commented Sep 18, 2019

Can you try the code from #1979, which I just created a couple hours ago? That combined with my previous and already merged pull request #1928 should fix your issues.

@Luflosi
Copy link
Contributor

Luflosi commented Sep 18, 2019

And don't map Alt+/ but alt+shift+: if those are the keys that you have to press.

@bew
Copy link
Contributor Author

bew commented Sep 18, 2019

And don't map Alt+/ but alt+shift+: if those are the keys that you have to press.

It's not a kitty mapping, but a keybind I use in zsh, so I have to use the keys that the terminal app will receive AFAIK

I will try your PR

@Luflosi
Copy link
Contributor

Luflosi commented Sep 18, 2019

a keybind I use in zsh

Oh, then my PR will most likely not do anything. Are you using macOS? If so see https://sw.kovidgoyal.net/kitty/conf.html#opt-kitty.macos_option_as_alt.

@bew
Copy link
Contributor Author

bew commented Sep 18, 2019

No, I'm using Archlinux (will add this in OP done)

@Luflosi
Copy link
Contributor

Luflosi commented Sep 18, 2019

Then I don't know why this isn't working, sorry.
Maybe @kovidgoyal knows why.

@kovidgoyal
Copy link
Owner

That's likely because the key press is alt+shift+/ not alt+/

There is no alt+shift+/ key combination in traditional terminal keyboard protocols, so you get nothing. I suggest mapping alt+: instead of alt+/ in your shell config. Ar if you dont want to do that, use send_text in kitty.conf to map alt+shift+slash to \x1b\x2f which IIRC is alt+/

@bew
Copy link
Contributor Author

bew commented Sep 19, 2019

I think there is a misunderstanding here @kovidgoyal

There is no alt+shift+/ key combination in traditional terminal keyboard protocols

I'm aware of that, but I'm not trying to map alt+shift+/, but alt+/. With the french keyboard layout it happens to require the shift key to type the slash, but I don't expect the terminal app (like zsh) to receive the shift key

The keybind in my zsh config is something like:

bindkey '^[/' some_function1
bindkey '^[#' some_function2

(with ^[ replaced with an actual escape char)

==> Note that in urxvt these 2 keybinds (alt+/ and alt+#) works correctly without any special configuration.

@bew
Copy link
Contributor Author

bew commented Sep 19, 2019

Ok, actually I'm having this issue with Alt + most french-related keys:

The french keyboard looks like this (image from wikipedia):
azerty

As you can see, the key with 1 (let's call it K1) gives & when pressed directly, and 1 when pressed with shift.

Here is a comparaison between urxvt and kitty:

On each terminal, I press the key K1 (the key with the 1 on it, see above) using some mods:

  1. K1 only
  2. Shift + K1
  3. (Ctrl+V then) Alt + K1
  4. (Ctrl+V then) Alt + Shift + K1

With a space between each step for clarity.

  • On urxvt terminal, I get & 1 ^[& ^[1
    Which corresponds to the french layout.

  • On kitty terminal, I get & 1 ^[1 ^[!
    Which corresponds to the french layout for the steps 1. and 2., and wrongly corresponds to the english layout for the steps 3. and 4.

Do you see my point?

@kovidgoyal
Copy link
Owner

Just add the send_text command I mentioned to kitty.conf and you will be
fine.

@bew
Copy link
Contributor Author

bew commented Sep 19, 2019

Just add the send_text command I mentioned to kitty.conf and you will be fine.

That would be a workaround, but the core behavior is still wrong, don't you agree?

Adding custom mappings with send_text in kitty.conf for all Alt+french_symbol_key seems wrong to me

@kovidgoyal
Copy link
Owner

Well, you are welcome to come up with a patch to fix it. I dont know of
a robust way to do so. One cannot both support binding a shortcut like
shift+alt+key and also support mapping shift+alt+key to alt+shifted_key
automatically. The toolkit reports only one of these. For US ASCII keys
kitty works around it by manually mapping well known shifted keys. For
arbitrary keyboard layouts that cant be done.

Well technically one can, but it requires changes to the
toolkit to report both key event types based on the current keyboard
layout. That is way too much work, at least for me.

@bew
Copy link
Contributor Author

bew commented Sep 19, 2019

After reading #606 and xkbcommon/libxkbcommon#61 I think I start to understand the problem and the general logic behind the current kitty behavior regarding these "complex" keybinds 🤔

Now reading the code of the function glfw_xkb_handle_key_event in glfw/xkb_glfw.c, I currently don't understand the relationship between the xkb keysym and the final glfw key:

  • Do the key events comes from xkb and converted somehow to glfw? (then why can't we use the resolved keysym using xkb?)
  • Or do the key events comes from glfw directly? (then why are you manipulating xkb-related stuff?)

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 19, 2019 via email

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

To fix this properly one would need to report both things
to kitty and re-write the kitty key handling code to make use of that
extra information.

That is something I'd love to have too, and I'm willing to try to implement this if it's ok with you.


I've been reading the input implementation in alacritty, they are not using glfw but a bunch of rust libraries that separates the GL drawing (glutin) from the window creation/input handling (winit), which directly exposes the physical scancode, mods, and the virtual key code after OS keyboard layout application.
→ So basically the data you described as the proper solution above 👌

From what I understand, kitty embeds a version of glfw in the root of the repository (in glfw/) + a few glue/wrapper files used by kitty to interact with glfw (xkb_glfw.{c,h}, dbus_glfw.{c,h}, ibus_glfw.{c,h}), right?

glfw currently reports only one thing to kitty. So kitty has no way of knowing which one to use. It uses the former not the latter, as that is what glfw sends.

Does that mean that glfw actually does not send both? / there is not way to get both?
OR that the wrapper you did around glfw's X input handling currently only reports this?


Also, should I open a new issue for this? I guess so? (to put discussion and implementation questions there)

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 22, 2019 via email

@Luflosi
Copy link
Contributor

Luflosi commented Sep 22, 2019

If you get this working for Linux I'll try to make it work for macOS.

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Thanks @Luflosi !

@kovidgoyal What is the state param given to _glfwInputKeyboard (then the keyboard callback) ?

@kovidgoyal
Copy link
Owner

It is used by the IME subsystem, for example for writing east asian
text. It is documented in glfw3.h

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Indeed thanks
Note: The word state is missing in these docs (glfw3.h & glfw-wrapper.h):

- *  @param[in] Used for Input Method events. Zero for normal key events.
+ *  @param[in] state Used for Input Method events. Zero for normal key events.

For the struct, it seems that the public GLFW API never passes a non-opaque struct to the caller / callback / user..
So I figured I would make a struct in the internals of glfw, pass it as an opaque struct to the callback and add a few methods (one for each field?) to extract the data from the struct from the callback, and re-create a struct (mirroring the one in glfw) in kitty's codebase to easily pass all the information around, does that sound right to you?

fyi, the struct I'm going to go with looks like this for now (pretty much WIP):

struct _GLFWkeyevent
{
  GLFWwindow *win;
  int scancode;
  int action;

  int key;
  int mods;

  int virtual_key; // default: ?
  int virtual_mods; // default: 0
  // or: os_key / resolved_key / ?
  // or: os_mods / resolved_mods / ?

  char text[64]; // default: empty

  int ime_state; // FIXME: store it here ?
};

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 22, 2019 via email

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

I think for a first PR just create a struct that passes exactly the same
info as the existing callback gets. We will add/modify the struct later
as needed.

given the glfw is now just a part of kitty, I dont particularly mind exposing a struct directly.

Ok I'll do that.

What is the reasoning behind glfw3.h vs glfw-wrapper.h ?
Do I duplicate the struct definition in the 2 ? Would it be possible (here or later) to include glfw3.h in the wrapper to avoid the duplications?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 22, 2019 via email

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Nevermind, diffing the two gave me the reasoning.

I still think it would benefit to include glfw3.h in the wrapper, and simply add all the wrappers after it.

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Oh ok, I don't touch it then (I'll add a line saying that this file is autogenerated though, to avoid thinking about this later)

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 22, 2019 via email

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 22, 2019 via email

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Since the rest of the wrappers have to be generated anyway, what do
we gain?

Sorry it wasn't clear.. Kitty's codebase always includes glfw-wrapper.h (but not glfw3.h), so If I add the new struct GLFWkeyevent to glfw3.h, I won't be able to reference its fields in Kitty's codebase directly

I can change the python generator to keep the full definition of the struct (based on a special comment or something) or include one into the other (and can strip the opaque type definitions from the generator), or I'm I missing something?

@bew
Copy link
Contributor Author

bew commented Sep 22, 2019

Actually, putting the new type among the rest of the opaque types generated it for the wrapper as well, so I guess it's fine.

@bew
Copy link
Contributor Author

bew commented Sep 24, 2019

So I got something working (extract the struct, change everywhere), still a few things to change I think, I'll open a PR soon!


Forget the following, let's continue the discussion on #2000 !

I noticed a change between 0.14.4 & current master for the second example I gave, using Alt-& which should give ^[& but was giving ^[1 (using the fallback keymap).
Since @Luflosi's e619eb9c, the & ampersand has now a glfw key for it (GLFW_KEY_AMPERSAND), and the behavior is now the same as the alt+shift+: keybinding (but without the shift problem): nothing is output on the terminal

Press scancode: 0x40 clean_sym: Alt_L composed_sym: Alt_L mods: none glfw_key: 342 (LEFT ALT) xkb_key: 65513 (Alt_L)
on_key_input: glfw key: 342 native_code: 0xffe9 action: PRESS mods: 0x0 text: '' state: 0 sent key to child

Press scancode: 0xa clean_sym: ampersand composed_sym: ampersand mods: alt glfw_key: 38 (AMPERSAND) xkb_key: 38 (ampersand)
on_key_input: glfw key: 38 native_code: 0x26 action: PRESS mods: 0x4 text: '' state: 0 sent key to child

I think it's because there is nothing in text, because this line:

if ( ((GLFW_MOD_CONTROL | GLFW_MOD_ALT | GLFW_MOD_SUPER) & sg->modifiers) == 0) xkb_state_key_get_utf8(sg->state, code_for_sym, key_event.text, sizeof(key_event.text));

prevents getting the text when ctrl / super / alt is pressed (but then I don't know why it works when I do a simple keybind like Alt+x).

I tried to remove the condition, but now I get & when I press Alt+&, the alt (serialized as ^[) is not sent... do you have any idea why? Maybe it has something to do with keys.py ?

Hopefully all these issues will go away with my later work to add physical keys + 'resolved' keys detection....

@kovidgoyal
Copy link
Owner

kovidgoyal commented Sep 25, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants