#1141 rewrite fix for non-latin keyboard shortcuts

[nyabkun]

If a key binding contains non-ascii code, it will change the keyboard group and retry several times.

Here is SSCCE.

Test Code [Edited]

from lib.gibindings import Gdk
from lib.gibindings import Gtk

def is_ascii(s):
    return s and all(ord(c) < 128 for c in s)

def translate(hardware_keycode, state, group):
    # We may need to retry several times to deal with garbled text.

    keymap = Gdk.Keymap.get_default()

    # distinct
    it = list(set([group, 0, 1, 2]))

    ok_to_return = False
    keyval = None
    keyval_lower = None

    for g in it:
        res = keymap.translate_keyboard_state(
            hardware_keycode,
            Gdk.ModifierType(0),
            g
        )

        if not res:
            # PyGTK returns None when gdk_keymap_translate_keyboard_state()
            # returns false.  Not sure if this is a bug or a feature - the only
            # time I have seen this happen is when I put my laptop into sleep
            # mode.

            continue

        keyval = res[1]

        # consumed_modifiers = res[4]

        lbl = Gtk.accelerator_get_label(keyval, state)

        if is_ascii(lbl):
            ok_to_return = True
            break

    if not ok_to_return:
        print('translate_keyboard_state() returned no valid response. '
                       'Strange key pressed?')

        return None, None, None, None

    # We want to ignore irrelevant modifiers like ScrollLock.  The stored
    # key binding does not include modifiers that affected its keyval.
    mods = Gdk.ModifierType(
        state
        & Gtk.accelerator_get_default_mod_mask())

    keyval_lower = Gdk.keyval_to_lower(keyval)

    # If lowercasing affects the keysym, then we need to include
    # SHIFT in the modifiers. We re-upper case when we match against
    # the keyval, but display and save in caseless form.
    if keyval != keyval_lower:
        mods |= Gdk.ModifierType.SHIFT_MASK

    # So we get (<Shift>j, Shift+J) but just (plus, +). As I
    # understand it.

    accel_label = Gtk.accelerator_get_label(keyval_lower, mods)

    return keyval, keyval_lower, accel_label, mods


def test(hardware_keycode, state, expected_accel_lbl):
  keyval, keyval_lower, accel_lbl, mod = translate(hardware_keycode, state, 0)

  print(f'#### accel_lbl : {accel_lbl}')
  assert expected_accel_lbl == accel_lbl

test(220, Gdk.ModifierType.SHIFT_MASK, 'Shift+Backslash')
test(65, Gdk.ModifierType.SHIFT_MASK | Gdk.ModifierType.CONTROL_MASK, 'Shift+Ctrl+A')
test(65, Gdk.ModifierType(0), 'A')

Test Code Output

#### accel_lbl : Shift+Backslash
#### accel_lbl : Shift+Ctrl+A
#### accel_lbl : A

[houndci-bot]

line too long (87 > 79 characters)

[houndci-bot]

line too long (92 > 79 characters)

[houndci-bot]

line too long (80 > 79 characters)

[houndci-bot]

expected 2 blank lines, found 1

[houndci-bot]

Black would make changes.

[jplloyd]

Thanks! You can ignore the "Black would make changes" warnings.

I will test this tonight (CET) and I will try to do a better job than when I tested the last PR. On the other hand, merging the previous PR is what lead to getting feedback so swiftly, so not altogether a bad thing.

[nyabkun]

If there is a problem with the formatting or the location of the newly created file, please point it out to me or you can fix it directly.

[jplloyd]

This resolves the Shift-issues and other related issues that were identified in #1141, but there are a couple of issues left to tackle.

For example, users of German and nordic layouts normally expect to be able to set and use shortcuts with characters like ö and ä, whereas this patch will make those turn into ; and : which are located on other keys for those layouts.

I will take a proper look at this issue tomorrow (Saturday), and dig into how applications like GIMP deal with this issue.

[nyabkun]

Thanks,

Gimp / Inkscape doesn't do anything special with the Shift key, and the display seems to be as it is without the Shift key (for special character like *, ~). At least that's the way it was in my environment.

I wonder if it's a bug in GTK that the keymap is unstable when multiple IMEs are registered.

[jplloyd]

Couldn't find anything about bugs with multiple input methods on the gtk tracker, but it's not impossible that it's a gtk issue (or something that simply isn't supported). I think it's more likely a problem with our implementation though.

Based on how things work in GIMP and Krita, I think the behaviour we want is to prioritize the active/primary layout when setting and using shortcuts, and only fall back to other groups if no shortcut is found for the prioritized group.

For example, in the Russian layout I'm testing with, the W key on my keyboard produces Ц. If I create a shortcut for Shift+W I would expect Shift+Ц to trigger that shortcut, unless I had another shortcut bound to Shift+Ц, which would then take priority. Would the expectation be the same for Japanese shortcuts, or do those usually work in some other way?

[nyabkun]

Thank you for the reponse.

On a Japanese keyboard, we basically use Romaji to type hiragana, and no one sets up keyboard shortcuts using non-ascii characters.

I think the behaviour we want is to prioritize the active/primary layout when setting and using shortcuts, and only fall back to other groups if no shortcut is found for the prioritized group.

In fact, that's what I was trying to do, but the code in the following part may have rejected it, causing it to fall back into a different group and not show the Ц.

def is_ascii(s):
    return s and all(ord(c) < 128 for c in s)

Also I think the problem of garbled characters when setting key shortcuts can only be handled by input character / hardware keycode which is unstable.

The problem would be solved if the behavior when setting key shortcuts were the same as in GIMP and Krita, but special character key shortcuts would not be displayed with "Shift+".

If we make the display like GIMP, there is a possibility that the shortcuts currently set will not work. (idk for sure)

---

If you use this code instead of is_ascii, it could possibly work.

        #if is_ascii(lbl):
        if keyval < 1000:
        # dirty way of checking validity
        # 1000 is probably beyond the upper limit of keyval?
            ok_to_return = True
            break

---

I modified the test code I put in the first post to work in standalone as well.
Sorry, was not SSCCE.

[jplloyd]

Ok, very good. Tomorrow evening I will take a more thorough look at the [relevant gdk documentation] (https://developer.gnome.org/gdk3/stable/gdk3-Keyboard-Handling.html) and build some minimal examples.

Unfortunately I don't think the modified keyval check is enough, as for example the keyval of the Ц key was over 1400 (I don't think a simple comparison will work, in the general case). There are also dead keys, for writing things like é è ô and similar, which should probably not be bound to shortcuts.

We should remember that the original code was written nearly 11 years ago, and many things have changed or been added to the gdk/gtk APIs since then. There probably is a better way of handling the keyboard shortcuts in general. Of course, as you said, if we change the setup we'll have to make sure to avoid breaking compatibility with existing shortcuts stored in user settings.

[nyabkun]

Thank you for all your research.

I was not able to reproduce this garbled bug in 2.0.1, so it seems that this may be a bug that occurs only in the development environment. ( python mypaint.py )

If there seems to be no good solution, it might be better to simply rewind my previous PR. #1139

I can manage to adjust it in my development environment.