More general/unified approach to keybindings

[camoz]

Description

I just tried to add a keybinding for q to exit satty, but apparently this is not possible. Also I noticed that keybindings are currently defined in two places: the [keybinds] section and the [general] section (namely the actions-on-* settings).

So I thought it would be nice to unify the approach to keybindings and have them only defined in the [keybinds] section, e.g. by adding Enter, LeftMouse, RightMouse, etc as possible "keys", and exit, save, etc as possible "actions". AFAICS this should be possible in a backwards compatible manner.

I think it would also be nice to have the mappings defined as keybind = action instead of action = keybind, as generally one keybind can only have one action, but one action can have multiple keybinds associated with it (e.g. Esc and q could both be mapped to exit). But this is probably not doable in a backwards compatible manner and not so important I think.

[RobertMueller2]

In the long run, I think shortcuts should be bindable to actions/action chains or tools or colours etc. We kinda need to do this for #89. Backwards compatibility is important, though.

[robertwidfen]

A first version is here: https://github.com/robertwidfen/Satty/tree/feat_generic_key_bindings

Please comment & test ;-)

The description of the change is below

Changes syntax for [keybinds] section in config file to

"key" = "tool-or-command"

Also adds:

• commands for size selection
• automatic hint generation for all buttons

Removes:

• layout independent key binding - instead bind what you warnings
• single letter binding restriction

Old style is still supported but will generate deprecation warnings.

The old code always sent key event into the IME handling before
they were processed as key event. Thus there were two places for
bindings, bindings with a modifier and single letter bindings.
But IME is for text input, thus it is now only activated for
the text tool.

[RobertMueller2]

Well done, and looks sound at first glance, and it's probably advanced enough to open a PR, if you ask me. Haven't had any opportunity to test yet, might take a few days.

One remark regarding the actions for enter and escape, I think in the future we may want to have something like named action chains (on the back of #89). E.g. something like actionchain1 which runs a shell command and then quits. For that, we'd probably want to refer to them in a dynamic way in the shortcuts, e.g. instead of
"<Control>q" = "run-actions-on-escape"
something like
"<Control>q" = "action-chain:escape-actions"

That's not requesting a code change, after all we don't know if #89 is ever implemented. And whoever does that may want some freedom regarding the syntax. But I'd like to ensure that run-actions-on-escape and run-actions-on-enter would be considered experimental/temporary so we could easily replace them later once #89 is done.

There might be more remarks in a review, although probably not that many. Who knows. it's a lot of code 😄

[robertwidfen]

You are right about #89, actually I have seen it - you did a good maintainer work by linking related issues! 🤗

I was thinking about touching that part, as with generic bindings it seems not to make sense to have actions with a binding (escape/enter) in the name. But I did not want to break/change too much.

IMHO currently it will only break layout-independent keys, but they are critical anyway at the moment as the code does not do a two-phase check, i.e. first check all normal bindings and only then as fallback the layout-independent ones (see #508). So removing layout-independent keys will definitely break binding for those that use them, but the generic bindings allow for solving the original issue - shortcuts not available on some keyboard layouts - IMHO in a better and safer way.

I also need to review the changes my self again 😇 - "pixelate" slipped into README.md 🙃 the change was first based on my WIP and then it was as larger rebase, merge & and partly squash of the other keybinding changes that I had done before...

[camoz]

Just a small note that I'm currently busy writing my thesis and cannot test. If this is still open after I'm done with my thesis I will test and get back to you. In any case, thank you for your work!

[robertwidfen]

This will take some more time before merging. It is a breaking change for people which use the layout-independent bindings - but layout-independent bindings as implemented now also can cause trouble like in #508.

It would be great if somebody using layout-independent bindings on a non-latin keyboard could test/comment on this. ☺️