Allow key modifiers in HOTKEY configuration setting

bk138 / gromit-mpx

Gromit-MPX is an on-screen annotation tool that works with any Unix desktop environment under X11 as well as Wayland.

GNU General Public License v2.0

1.01k stars 84 forks source link

Allow key modifiers in HOTKEY configuration setting #166

Closed godmar closed 2 years ago

godmar commented 2 years ago

Currently, it is not possible to specify modifiers for the hotkey that toggles Gromit on. Usually, this is ok: the code snoops for all modifiers, and then applies different actions (toggle, clear, etc.) based on the modifiers (such as CTRL, SHIFT, etc.)

However, if an input device (such as a Dell active pen) triggers keystrokes with modifiers, then omitting the modifier in the .cfg file will not be compatible with the required keybindings implemented in 6501486.

This PR strips the modifiers before trying to convert the symbolic keyval to a keycode. However, the modifier is kept in the symbolic keyval so it can be added correctly when installing the keybindings.

Comments are welcome - is this the right way to do this?

godmar commented 2 years ago

ps: sorry for leaving cd1a9c2 in this PR, it's more of a proposal anyway.

godmar commented 2 years ago

To add, this patch

will not change the existing behavior if the HOTKEY= setting does not contain modifiers
if the HOTKEY= value does contain modifiers, they will be stripped for the purposes of determining the keycode, but they will be retained for the purposes of the GNOME-specific hotkey bindings added via 6501486d.

bk138 commented 2 years ago

This looks OK as a PoC. To get ready to merge this, I'd like you to please

clean up the PR to contain only one commit
add a refined version of you explanations in https://github.com/bk138/gromit-mpx/pull/166#issue-1344747579 and especially https://github.com/bk138/gromit-mpx/pull/166#issuecomment-1221089195 to the commit message and as a /* .. */comment in front of the code change.

In short: :+1:

godmar commented 2 years ago

I'm not sure this PR doesn't add too much technical debt and confusion for the future.

I don't like that the specification of the HOTKEY now allows modifiers which are ignored by part of the code and honored by another part of the code. This is just asking for confusion in the future.

The entire compositor shortcut key business seems like a hack that based my understanding will become obsolete as soon as they support a standardized API.

bk138 commented 2 years ago

Alright, I had some more thoughts about it as well and I think you're right. Let's keep things simple.