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

keycodes_from_xmodmap returns one-to-one mapping instead of one-to-many #316

Open
nkruglikov opened this issue Oct 19, 2024 · 7 comments
Open
Labels
bug Something isn't working

Comments

@nkruglikov
Copy link

Background

I want to define a key mapping to control Spotify via DBus. For that, I want to link XF86AudioPlay key (Fn+F9 on my keyboard) with a spawn("dbus-send ..."). However, it didn't work: the key press did nothing.

The issue

Digging into the code, I discovered the following.

  • parse_keybindings_with_xmodmap works by getting a mapping from String to u8 from keycodes_from_xmodmap and using that to convert key names like XF86AudioPlay to key codes like 172.
  • keycodes_from_xmodmap executes xmodmap -pke. This command dumps keycode mappings in lines of the following format:
    ...
    keycode  52 = z Z Cyrillic_ya Cyrillic_YA
    ...
    
    The above line would be turned into the following pairs:
    ...
    (z, 52)
    (Z, 52)
    (Cyrillic_ya, 52)
    (Cyrillic_YA, 52)
    ...
    
    Then this pairs are collected into a HashMap.
  • On my system, xmodmap -pke | grep -i xf86audioplay outputs the following:
    keycode 172 = XF86AudioPlay XF86AudioPause XF86AudioPlay XF86AudioPause
    keycode 208 = XF86AudioPlay NoSymbol XF86AudioPlay
    keycode 215 = XF86AudioPlay NoSymbol XF86AudioPlay
    
    This means that keycodes_from_xmodmap would first turn it into the following pairs:
    (XF86AudioPlay, 172)
    ...
    (XF86AudioPlay, 208)
    ...
    (XF86AudioPlay, 215)
    ...
    
    and then, during collect, just into a mapping
    XF86AudioPlay => 215
    
  • With xev utility, I confirmed that pressing "Play" button of my keyboard emits keycode 172. Thus, it doesn't get captured by Penrose.

Possible fix

Possible fix would modify the logic of keycodes_from_xmodmap so that it returns Result<HashMap<String, Vec<u8>>> instead of Result<HashMap<String, u8>> and then adapting parse_bindings_with_xmodmap / parse_binding functions. I am not sure how popular is this problem, though, and whether changing the signature of keycodes_from_xmodmap worth it. In any case, I am open to implementing it.

System details

I run Penrose on Arch Linux with startx.

setxkbmap -print -verbose 10 returns this:

Setting verbose level to 10
locale is C
Trying to load rules file ./rules/evdev...
Trying to load rules file /usr/share/X11/xkb/rules/evdev...
Success.
Applied rules from evdev:
rules:      evdev
model:      pc105
layout:     us,ru
options:    grp:caps_toggle
Trying to build keymap using the following components:
keycodes:   evdev+aliases(qwerty)
types:      complete
compat:     complete
symbols:    pc+us+ru:2+inet(evdev)+group(caps_toggle)
geometry:   pc(pc105)
xkb_keymap {
	xkb_keycodes  { include "evdev+aliases(qwerty)"	};
	xkb_types     { include "complete"	};
	xkb_compat    { include "complete"	};
	xkb_symbols   { include "pc+us+ru:2+inet(evdev)+group(caps_toggle)"	};
	xkb_geometry  { include "pc(pc105)"	};
};

It is a fairly standard configuration with English / Russian layout switch on CapsLock.

My keyboard is Logitech G Pro:

Picture of the keyboard

image

@nkruglikov nkruglikov added the bug Something isn't working label Oct 19, 2024
@sminez
Copy link
Owner

sminez commented Oct 22, 2024

This has been mentioned previously I think? Strictly speaking it would be a breaking change to alter the return type of the function parsing keycodes and I'm not 100% confident that doing so in a way that didn't require changes in user code would be guaranteed to not alter existing use of the library. Having an alternative parse function that set up the keymap is probably the safe thing to add instead?

@nkruglikov
Copy link
Author

I think this is the closest issue: #228
Another one with a similar situation (key name has two possible keycodes), but not sure whether the original problem has the same cause: #277

Implementing a separate parsing function seems a nice thing to do in order to ensure backwards compatibility. I can try implementing it, would it be okay?

@sminez
Copy link
Owner

sminez commented Oct 22, 2024

If you're up for it then go for it! If you can get something working and tested then I'll take a look and have a think about how to write the docs to explain the need / difference between the two implementations.

@nkruglikov
Copy link
Author

Re: two implementations. What do you thing about the following solution: add deprecation warning to the old option, and use the new option in the examples? As far as I can see, there is no benefits in the old option apart from backwards compatibility, and it also has footguns like the one I encountered.

@sminez
Copy link
Owner

sminez commented Oct 22, 2024

Sounds good 👍

My only concern is that the odd behaviour of the current implementation is required for someone's current set up. Unlikely but I want to be careful nonetheless

@nkruglikov
Copy link
Author

I tried to implement it and quickly realized a problem with the idea of using one-to-many mapping. As I said earlier, I use two layouts: English (US) and Russian. The problem is, they have common symbols assigned to different keys. For example, ., called period by xmodmap, is on a "slash" key if I use Cyrillic layout:

$ xmodmap -pke | grep period
keycode  60 = period greater Cyrillic_yu Cyrillic_YU
keycode  61 = slash question period comma

So, under the old system, if I use M-period binding, it would actually assign to "slash" key instead of "period" key. This is not good, obviously, and the system will pick the Cyrillic option only because the line for it in xmodmap output is lower. For some symbol names, for example, colon, the situation would be reversed.

However, under the new system, the situation would also be bad! Because M-period will be assigned to both the "period" and the "slash" key.

Right now, I am a bit unsure how to implement a correct solution. For me (and my bi-layout-al peers) the expected flow is to assign bindings in terms of QWERTY and expect them to be the same (physically) even when layout is switched. But how to correctly extract "the primary" layout from xmodmap, I am not sure. Even worse, Arch Linux Wiki states that:

xmodmap is not directly related to XKB; it uses different (pre-XKB) ideas on how keycodes are processed within X. In particular, it lacks the notion of groups and types, so trying to set more than one keysym per key is not likely to work. In general, except for the simplest modifications of keymaps or pointer button mappings, xkbcomp(1) should be used instead.

Right now I am trying to educate myself on these systems and proper ways to use them.

@sminez
Copy link
Owner

sminez commented Oct 23, 2024

Ah...this is the sort of thing I was worried about 😅

There is always the escape hatch of best-effort parsing the keybindings with xmodmap and then explicitly inserting additional keys into the map to account for the ones that aren't handled correctly. But that's obviously not ideal if it can be avoided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants