dvdhrm / kmscon

Linux KMS/DRM based virtual Console Emulator
http://www.freedesktop.org/wiki/Software/kmscon
Other
433 stars 81 forks source link

Ctrl+Key doesn't work with non-ascii-like keyboard layouts #39

Closed bluetech closed 11 years ago

bluetech commented 11 years ago

Suppose my --xkb-layout is "us,il", and the active layout is "il". This layout produces non-ASCII keysyms in the base level. In this case typing e.g. Ctrl+l (that is the lower case 'l' keyboard key / xkb keycode) should still clear the screen, etc. This works in xterm, urxvt and libvte (though strangely Ctrl is the only modifier which gets this special treatment).

libvte had these bugs about this issue: https://bugzilla.gnome.org/show_bug.cgi?id=375112 https://bugzilla.gnome.org/show_bug.cgi?id=589557

They solved it here: http://git.gnome.org/browse/vte/commit/?id=d64e28b3290aaf034ec37bbf4524ef78adbdfb2e Which claims xterm does the same. It seem to work, so I think kmscon should do the same.

The equivalent loop in xkbcommon would look something like this: https://gist.github.com/3833422 This is simple enough, and I've made sure it works as expected (it requires some relatively new API though). However the tsm_vte does not facilitate doing this directly like the libvte code above does (it doesn't have access to the xkb_state), so I didn't attempt a patch myself.

dvdhrm commented 11 years ago

So the issue basically is: ctrl+c should always work even though the "c" key may be labeled "whatever"? So on a keyboard that has no key which generates XKB_KEY_c, ctrl+c sould still work?

So what your small helper does is iterating through all layouts in the user's keymap and finding the first layout that maps a given keycode to exactly one keysym which is in the ASCII range, right? What happens if the user has two layouts which both map those to the ASCII range but differently? I think this patch will always pick the first layout instead of checking the currently active layout first. Other than that it sounds reasonable. So what I will basically do is getting the ASCII-sym (via this function) for every reported key and just pass it together with the normal data to TSM. This loop does not look very heavy. The xkb functions just set the passed pointers to the right destination, they don't do any heavy work, do they?

So if the keysym that I get from the xkb_state is not an ASCII keysym, then I simply call this helper to get a possible ASCII keysym. In TSM, I will then use the ASCII keysym if CTRL is held. If the ASCII keysym is invalid, I will fall back to the normal keysym.

Thanks a lot for investigating. I actually only use "de,en" so I would have never noticed. Regards David

bluetech commented 11 years ago

Yes, that's the problem. Just --xkb-layout=us,il --xkb-option grp:menu_toggle, typing the MENU key to switch the group, typing Ctrl+c should work.

I also find looping through the layouts like this pretty weird, and besides, if you just write --xkb-layout il for example you're essentially stuck. But that's how xterm and gnome-terminal work, so we probably shouldn't diverge? It might be wise to implement a few "safety" shortcuts by looking at event->keycode instead of event->keysym? But that's off topic here.

Yes, you should definitely look at the normal keysym first, and if it doesn't work, you would look at the result of the helper. Adding the ascii to the event might do some unnecessary processing, but you would normally skip it (I would guess people are typing ascii most of the time; it is a terminal after all). The only bit that does any work at all is xkb_state_key_get_level, but it's pretty much nothing as well.

Thanks!

dvdhrm commented 11 years ago

Hi Ran

I just pushed some patches that modify input handling of kmscon and I also fixed this issue by passing an "ascii" value for each input-event. This doesn't affect performance on my machine so I think this is the right fix.

See 33ebc01441f9d4f8a7983aef8694651b42e043c6

Thanks for the report! David