#61 Keyboard shortcut preferences

[Ark]

I don't have that much time right now, so I must be brief.

1. Thank you for that feature! It even serves as documentation (nice overview) for the user.
2. What is the difference between "Fullscreen" and "Toggle fullscreen"?
3. You made a distinction between "dynamic" and non-dynamic page flipping, as in "Next page" and "Next page (dynamic)". What about the other actions, e.g. "Go forward ten pages"? I think there should be two groups, "Navigation" and "Navigation (reading direction aware)" with all "Navigation" actions in both groups. An exception is "Go to page" that should only be found in the reading direction unaware group.
4. How about a hint that says that "dynamic" (or, as proposed, "reading direction aware") actions must be entered for left-to-right reading direction in this dialog?
5. Another tip that says "'Backspace' removes a shortcut".
6. How to re-assign the BackSpace key, e.g. to "Previous page", since backspace removes a shortcut? Besides that, I cannot bind keys such as Right, Left, Up, Down, KP_Right, KP_Left, KP_Up, KP_Down.
7. If you don't bind <Primary>KP_[Number] explicitly then it is implicitly bound to the same action KP_[Number] is bound to. That is, both KP_5 and <Primary>KP_5 scroll to the center of the image.
8. If I try to add and/or remove ambiguous key bindings, I get the following errors:
   Traceback (most recent call last): File "/home/ark/mcomix-code_r915/mcomix/keybindings_editor.py", line 102, in on_accel_edited titer = action_treeiter_map[affected_action] NameError: global name 'action_treeiter_map' is not defined Traceback (most recent call last): File "/home/ark/mcomix-code_r915/mcomix/keybindings_editor.py", line 94, in on_accel_edited affected_action = self.keymanager.edit_accel(action_name, new_accel, old_accel) File "/home/ark/mcomix-code_r915/mcomix/keybindings.py", line 218, in edit_accel self._binding_to_action.pop(ob) KeyError: (98, <flags 0 of type GdkModifierType>) Traceback (most recent call last): File "/home/ark/mcomix-code_r915/mcomix/keybindings_editor.py", line 121, in on_accel_cleared self.keymanager.clear_accel(action_name, accel) File "/home/ark/mcomix-code_r915/mcomix/keybindings.py", line 237, in clear_accel self._action_to_bindings[name].remove(ob) ValueError: list.remove(x): x not in list
9. What about a button to reset all shortcuts?
10. Bug: Shortcuts can override mnemonics. If x is bound to an action, then <Alt>x is bound to the same action as well. Thus, I cannot say <Alt>e to open the Edit menu if e or <Alt>e is bound to an action.
11. Bug: I can bind <Shift>f to "Fullscreen" (for example) in the dialog. But when I press <Shift>f it doesn't switch to fullscreen.

[Ark]

I have something to add:

1. According to the Wiki ([Keybindings]) you can hit CTRL+Left (or similar) to go back/forward only one page when in double page mode. Actually, you can do so using only the CTRL key as a modifier. (You don't need to hold the SHIFT key in order to go back. This is contrary to the Wiki.) Besides this, there is no entry in the "Shortcuts" preferences for these "only one page" actions.

Anyway, thank you for your work, especially the "Fit to width" bug fix. I don't have much time for coding at the moment, so I'm only testing.

[Oddegamra]

My cat killed my laptop while I was writing a reply by stepping on the power button. I'm not even kidding.

1. You're right, those are one and the same action. Merged into one.
   3-5. This is mainly due to my laziness. Added to the todo-list.
2. The UI element for changing hotkeys is a stock GTK control (CellRendererAccel). The docs unfortunately do not say anything specific. For example, ALT+Left can be bound, but Left alone is ignored. Strange.
3. This is the "fault" of the exec code in keybindings.py. Some keys cannot be pressed without modifier on certain layouts, for example, on a QWERTZ keyboard, to get \, you'd have to press R-ALT plus "-". But once the key is bound, GTK stores "backslash", not ALT + some other key. So, when key_pressed is executed, it receives ALT + backslash as argument, not just backslash. It doesn't realize that ALT is a part of the combination needed to even get to backslash, and doesn't execute the keybinding because it does not contain ALT. To get around this problem, MComix tries to ignore modifiers if no other binding succeeds before. Of course, this also leads to ALT+KP5 triggering KP5 if no explicit action is bound to ALT+KP5. Not sure if there is a way to get around this problem.
4. Great, I missed that. Fixed.
5. I've been thinking about something like that as well. On the todo-list.
6. Big problem. The only thing I could think about right now is suspending key handling while a menu is opened.
7. Strange, I have no problems with that.
8. CTRL working with all sorts of paging combinations is somewhat intentional. Maybe the keybinding page isn't concise enough.

[Ark]

An idea that just came to my mind: Maybe you can copy the way IBus handles keyboard input: https://code.google.com/p/ibus/ (Ibus should be listed in the average Linux distribution's package lists.)

[Oddegamra]

After digging around some in the GTK docs, I might have finally solved the problem with keyboard modifiers being accepted even though they should not be. This leaves the mnemonics overriding problem. I changed the keyboard handler callback from connect to connect_after - that is, GTK should handle whatever it can and only pass on keys that did not have any specific function. Does that work for you? It seems to to the trick for me.

[Ark]

At r923. Let's see …

1. Thanks again!
2. fixed
3. unchanged
4. unchanged
5. unchanged
6. unchanged
7. fixed
8. fixed
9. fixed
10. fixed
11. unchanged. <Shift>f works if and only if it is registered as "Key 1". To make things more complicated, the assignments for "Key 1", "Key 2" etc. get mixed up as soon as you register <Primary>F11 in the same line. (That is, if you set <Shift>f to be "Key 1" but add <Primary>F11 to the same action as well (e.g. "Key 3"), you should reopen the dialog and check the order.)
12. can't reproduce. Ctrl+Left doesn't work at all now, even with NumLock turned off (see below).
13. new(?) bug: If NumLock is turned on and you try to use the cursor keys (the normal ones, not their KP counterparts), the thumbnailer gets focused and you can scroll horizontally using Left and Right in some configurations, revealing an "uncolored" vertical line at the left edge of the thumbnailer (assuming "uncolored" means "almost white" and the thumbnailer background is set to "almost black"). "Some configurations" means having enough images to flip through so a scrollbar appears in the thumbnailer. Workaround: Turn off NumLock. (Needs to be turned on again every time you want to use the KP.)

Well, I think the overall result is better than before, but it still needs some improvements.

[Ark]

At r931.

1. unchanged ;)
2. ignored
3. unchanged
4. unchanged
5. unchanged
6. unchanged. I wonder how IBus solved this.
7. ignored
8. ignored
9. ignored
10. ignored
11. fixed
12. unchanged. Maybe it's easier (and more consistent) to give these actions their own respective keybindings.
13. fixed
14. new issue: For symmetry reasons, "First page" and "Last page" should be mapped to KP_Home and KP_End by default as well, respectively.
15. new bug: Attempting to start the slideshow using Ctrl+S results in this:

Traceback (most recent call last):
  File "/home/ark/mcomix-code_r931/mcomix/event.py", line 405, in key_press_event
    manager.execute((keyval, event.state & ~consumed & ALL_ACCELS_MASK))
  File "/home/ark/mcomix-code_r931/mcomix/keybindings.py", line 256, in execute
    return func(\*args, \*\*kwargs)
TypeError: toggle() takes exactly 2 arguments (1 given)

I tried to fix this myself, but I didn't fully understand the problem. Using the menu works as expected, though.

By the way, did you receive an e-mail for the following post? I did not. https://sourceforge.net/p/mcomix/feature-requests/75/#e688

[Oddegamra]

.6. Still no idea. Maybe subclassing the binding element and taking care of recording keystrokes ourselves.
.12. I came to realize that the CTRL+Key combination actually only worked because of that bug/feature you mentioned in #7. Now that said bug/feature is fixed, naturally, the CTRL key only works for either scrolling the mouse wheel or hitting one of the next/previous UI buttons. I agree that the best fix would be to explicitly define new key actions. Only laziness prevented me from actually doing this to this date. ;)
.14. They aren't? Good catch.
.15. Thanks, missed that one. The hotkey called a function that was only supposed to be called via click action callback. Well, having such a function public is probably bad design to begin with, but it makes it easier to remember/sync if a slidehow is running or not.

Oh, and I did receive an email for said item. I pretty much get emails for everything. Maybe your spam filter catches them?

[Ark]

.6. As I mentioned earlier, I wonder whether the code of the function __keycode_button_clicked_cb in https://github.com/ibus/ibus/blob/master/setup/keyboardshortcut.py might be helpful. (If you are not used to IBus, install ibus, start ibus-setup and try to modify the keyboard shortcuts shown in the "General" tab to see how it works. I looked at the code but I still don't get it.)

About that email thing: Well, actually, I don't know. I usually receive mails about almost everything that happens around this project. Strange. But thank you for the reply.

[Ark]

@Oddegamra: I experimented a bit with the "accel-mode" as it is documented on pygtk.org:

http://www.pygtk.org/docs/pygtk/class-gtkcellrendereraccel.html
http://www.pygtk.org/docs/pygtk/gtk-constants.html#gtk-cellrendereraccel-mode-constants

It looks like we might need both modes in parallel somehow. gtk.CELL_RENDERER_ACCEL_MODE_GTK works fine if you want to bind more "ordinary" action keys like letters or the Return key. gtk.CELL_RENDERER_ACCEL_MODE_OTHER is necessary if you want to bind keys like Cursor Up or Cursor Down. However, none of these modes accept the Backspace key, and there is another problem, too:

For example, if you need a shift key to type a question mark, using gtk.CELL_RENDERER_ACCEL_MODE_GTK will give you "question", whereas gtk.CELL_RENDERER_ACCEL_MODE_OTHER will give you "<Shift>question". Only "question" will work, though, at least for me.

I'm sorry, it really isn't that helpful, I guess. I'm still wondering how ibus managed this.

The patch switches the mode to gtk.CELL_RENDERER_ACCEL_MODE_OTHER so you can experiment with it, too.