[KOPlugin] Hotkey Shortcuts, add custom keyboard shortcuts

[Commodore64user]

what's new

This plugin allows users to configure and use hotkey shortcuts for various dispatcher actions within the application. It provides a flexible way to map keys to specific actions based on the device's capabilities and user settings.

features:

• Configurable hotkey shortcuts for cursor keys, page-turn buttons, function keys, and alphabet keys.
• Supports different key mappings for kindles, and other devices with physical keyboards.
• Allows users to enable or disable the use of the press key for shortcuts.
• Provides a menu interface for configuring and managing hotkey shortcuts.

key functions:

• init: Initializes the plugin, loads settings, and registers key events.
• onHotkeyAction: Handles the action triggered by a hotkey press.
• registerKeyEvents: Registers key events for the plugin based on device capabilities.
• overrideConflictingFunctions: Overrides existing key event registration functions to resolve potential conflicts.
• onFlushSettings: Flushes settings data if it has been updated.
• updateProfiles: Updates hotkey profiles when action names are changed.

usage:

• The plugin is automatically initialised and integrated into the application for devices that have a physical keyboard or are non-touch kindles.
• Users can configure hotkey shortcuts through the main menu interface.
• Hotkey actions are executed based on the configured key mappings and device capabilities.

screenshots:

• fix #7308, fix #12336.

---

This change is 

[yparitcher]

It might help to take some inspiration from Custom Multiswipes in gestures where the user can record a custom gesture, same here instead of having to list every combination of keys, you can just allow a user to chose his own key shortcuts.

https://github.com/koreader/koreader/blob/master/plugins%2Fgestures.koplugin%2Fmain.lua#L390

The trick is to open a dialog and register a handler for every type of key press, like is done for every type of gesture in the snippet below. Not sure if our current key code can handle this. https://github.com/koreader/koreader/blob/master/plugins%2Fgestures.koplugin%2Fmain.lua#L441

[yparitcher]

This would probably need to be extended: https://github.com/koreader/koreader/blob/master/frontend%2Fui%2Fwidget%2Fcontainer%2Finputcontainer.lua#L228

You can add a event for Input.group.Any

Also you need to deal somewhere with priorities (do shortcuts override default keybindings or not)? Gestures extensively deals with this for touch.

[yparitcher]

See this comment for how to store the key name in settings

https://github.com/koreader/koreader/blob/adbf3be6ec40872574684688f470e6bf0acb91a8/frontend/device/key.lua#L34

Tldr, you can just cast the key object to a string and it will be sane

[Commodore64user]

It might help to take some inspiration from Custom Multiswipes in gestures where the user can record a custom gesture, same here instead of having to list every combination of keys, you can just allow a user to chose his own key shortcuts.

as for this bit, great idea but i would prefer not to do that just now. I would be using this on kindle (3 and 4) so the 10 to 50-ish combinations are already plenty enough, for other devices that have keyboards though, it could be something to add in later.

Furthermore, I want to keep some key mappings (shift+alt+whatever) open in case of needing to hard code something. I also don't want to support single key shortcuts (besides press that is), as i want to implement a 'type to search' feature as well.

Also you need to deal somewhere with priorities (do shortcuts override default keybindings or not)? Gestures extensively deals with this for touch.

now this, i am aware of and i'm currently thinking about this very issue. I wanted to simply do something like (i know it’s not handling all cases there, i.e plugin being removed)

function ReaderToc:registerKeyEvents()
    local hotkey_plugin = self.ui.hotkeyshortcuts and not self.ui.hotkeyshortcuts.disabled
    if Device:hasScreenKB() and hotkey_plugin then
        self.key_events.ShowToc = { { "ScreenKB", "Up" } }
    elseif Device:hasKeyboard() and hotkey_plugin then
        self.key_events.ShowToc = { { "T" }, { "Shift", "Up" } }
    end
end

[yparitcher]

We can either do it easy or do it right :) To manual hardcode a few codes is easy but is asking for long term kludges as people add in new one esp with regards to priorties.

The right way would be to duplicate a lot of the gesture handling in input esp _ordered_touch_zones for priorities. This would allow converting the module specific handlers to registering with this plugin and calling events which simplifies code everywhere and automates the priorities. (Kind of the evolution gestures went through during its history) Instead of just adding to an array of key_events you would call something like registerTouchZones and co, but for keys and set a priority etc. This would then also allow you to unregister the key solving an issue you mentioned in one of the comments.

https://github.com/koreader/koreader/blob/adbf3be6ec40872574684688f470e6bf0acb91a8/frontend/ui/widget/container/inputcontainer.lua#L134

It will probably be a bunch of effort of reading the Gestures, Input, InputContainer, Key & DepGraph stuff and seeing which to adapt and tweek for key based input

[Commodore64user]

We can either do it easy or do it right :)

we? are you somehow suggesting that you are going to help get this to be the best it can possibly be? Because if that is the case, okay, we can try, you have the know-how. But if you are expecting me to figure it out by myself, and implement every single lesson learned by everyone that has ever worked on the gestures plugin here, well... you're setting yourself up for disappointment as that is just not going to happen.

If it comes to go big or go home, I am going home. But don't get me wrong, I am not saying that we should release something bad, what I am saying is, that I am not going to do all that work that has happened over many years and by many people (including yourself), either alone, or now (for this PR). So as I said, if anyone really wants to custom-create shortcuts (instead of using pre-defined ones), they can either wait, until I can get there some time in the future (post-this-PR) or grab a keyboard and help out.

It will probably be a bunch of effort of reading the Gestures, Input, InputContainer, Key & DepGraph stuff and seeing which to adapt and tweek for key based input

this is a very tall order (for me at least, a non developer I must add in case you don't know) for one man and little time, Rome was not build in one day. What I have here in this PR so far, is far more than what I personally need. So if worst comes to worst, I'll just run it locally and we can all go from having some sort of shortcut support to nobody having any, (except for me, because I will be running this) for better or worse.

So, making that clear, what are legitimate things that could be reasonably implemented here?

[Commodore64user]

To manual hardcode a few codes is easy but is asking for long term kludges as people add in new one esp with regards to priorties.

I don't think anyone (besides me) has in very many years hardcoded any type of key_events besides the bare minimum functionality. I also very much doubt that anyone is going to do so in future, especially with something like this PR. Now all one needs is to add more events to dispatcher. So I am not truly concerned about more modifier events and their potential future priority (although I might be wrong and it might bite me in the ass later).

[Commodore64user]

[  ERROR  ] spec/front/unit/filemanager_spec.lua:24: FileManager module should show error on non-existent file
[  ERROR  ] spec/front/unit/filemanager_spec.lua:39: FileManager module should not delete not empty sidecar folder
[  ERROR  ] spec/front/unit/filemanager_spec.lua:77: FileManager module should delete document with its settings

[Commodore64user]

@poire-z sorry to bring you back from hibernation. I was wondering if you had any idea why CI is giving those errors and how to solve them. Also, have a look around.

[poire-z]

You'll have more info for the why of these error above the bottom where they are summarized, using Ctrl-F to find the filename:linenumber ie.:

Also, have a look around.

Well, I have not enough brain bandwidth to dig into and appreciate your work. I'm somehow just happy it is fully contained in a plugin, and whatever you do in it won't bother most non-key-device users :) I also can't really estimate how much hack is happening with disabling core modules key registration, how much care/maintenance we'll have to give to it when tweaking the core stuff, etc...

[Commodore64user]

hoping @NiLuJe joins the dark side and has a look around here.

[NiLuJe]

Extremely unlikely ;).

[Commodore64user]

should I assume that the lack of interest means I should put it to rest?

[poire-z]

No. The fact that you managed to make it a standalone plugin (without touching core) is good. With the side effect that we don't really need to be involved and interested and looking for sabotage :) It just needs one of us to find the time and interest to give it an eye.

[Frenzie]

At a quick glance it looks a lot more appealing than the previous version. ^_^

[Commodore64user]

would anyone like to have a look at the Full refresh event, it doesn't appear to work... with shortcuts (at least)

[Frenzie]

The event is simply one that comes from Dispatcher?

[Commodore64user]

The event is simply one that comes from Dispatcher?

Yes, but even if you hardcode it (so it doesn't go through dispatcher), still won't work. I think i might have mentioned that early on the development of this idea (when it was just moving the bottom menu to the top, remember that? Feels like light years away)

[poire-z]

Nicer isn't it ? :) Thanks.

[Commodore64user]

Nicer isn't it ? :) Thanks.

For the record, i was bullied into making the change ;)

[Commodore64user]

Wondering if i could get this for Christmas 🎅 (or earlier ;) )

[poire-z]

Should you rename the plugin/hotkeyshortcuts.koplugin directory ? (If no: hotkeys plural, while it is singular in hotkeyshortcuts)

[Commodore64user]

Should you rename the plugin/hotkeyshortcuts.koplugin directory ?

No we shouldn't ;)

(If no: hotkeys plural, while it is singular in hotkeyshortcuts)

hotkeyshortcuts is a plural form. So, no issues there.

[Commodore64user]

commit 6bd7adb92968b4565cfd8fa887aad33055fb9a92 was heavily inspired by discussion #12702, does that seem like a reasonable approach?

[Frenzie]

Only if by "redundant" you mean "not implemented here", which is something very different than redundant.

[Commodore64user]

Only if by "redundant" you mean "not implemented here", which is something very different than redundant.

Well i meant, from a user perspective, both “pass through” and “nothing” (in the shortcuts context) do absolutely the same thing, a big fat nothing. So redundant from a user point of view.

[Frenzie]

Fair enough, but code comments are for programmers. The question isn't whether the two actions happen to be identical in this instance but why they are.

[Commodore64user]

first of all, sorry about all the rebasing happening lately. somehow my local branches got out of sync but should be okay now. I was wondering if anyone could have a look at this https://github.com/koreader/koreader/compare/master...Commodore64user:KOReader_fork:hotkey-test and "deem" it worthy on PRing. It is essentially tests (everyone's favourite task).

[mergen3107]

Continuing from https://github.com/koreader/koreader/issues/12744#issuecomment-2483645052

Feedback: AWESOME! Look at this beautiful Quick Menu called by ScreenKB+RPgFwd:

However, one shortcut didn't work for me: ScreenKB+Down for History (it was Book Map by default). It still summons Book Map.

Here is my hotkeys shortcuts settings file: hotkeyshortcuts.lua.txt

---

Also, ScreenKB+Press didn't work to replace it to Full Screen Refresh, it still defaults to add current page to location history.

---

Other than that, I only have questions/suggestions:

• Can each button have also long-press or double-press?
• Can Screensaver unlock gesture analogy be implemented for buttons?

Long-press might be really helpful to do something like 10 pages Forward/Back on long-press of page-turn buttons

---

Right now (without the PR), is there default key shortcut to fully refresh screen?

---

Also, inspired by default actions (https://www.mobileread.com/forums/showthread.php?t=152104), another shortcut might be added to input mode:

• ScreenKB + PgFwd to add a white space

---

Thank you @Commodore64user for making this PR! Really helpful

[Commodore64user]

@mergen3107 first of all, nice one, lovely QuickMenu.

I just tried using different actions with screenkb + down and it works just fine for me (on both K3 and K4), including the History event, looking at the file you uploaded, it looks like no changes have been made as it looks just like the default.lua file, hotkeyshortcuts.lua (found in the koreader/settings/hotkeyshortcuts.lua directory should be a json like file). it should look like this [after changes have happened]:

["modifier_plus_right_page_forward"] = {
    ["settings"] = {
        ["order"] = {
            [1] = "swap_right_page_turn_buttons",
        },
    },
    ["swap_right_page_turn_buttons"] = true,
},

could you go to the screenkb + down hotkey and set it to No action first and then assign it to History, see what happens then?

The Full screen refresh issue is known, and it seems unrelated to the hotkey itself but rather to the event. I've been seeking someone to investigate this further https://github.com/koreader/koreader/pull/12484#issuecomment-2422787161, but haven't had any luck so far... cough cough.

Can each button have also long-press or double-press?

Not at the moment. much like it was stated in the other FR you made, kindle does not support keyRepeat events so double press or long press is not happening anytime soon. Unless a generous soul were to refactor all that... fingers crossed

Can Screensaver unlock gesture analogy be implemented for buttons?

This is definitely doable. I can't make any promises, but once we get this merged, it's something I could look into. To be honest, it's not something I use, so it hadn't even occurred to me.

Long-press might be really helpful to do something like 10 pages Forward/Back on long-press of page-turn buttons

I agree, but again unlikely unless the key repeat thing is sorted out.

Right now (without the PR), is there default key shortcut to fully refresh screen?

No, that was a glaring omission on my part. I was focused on "navigation" and completely forgot to add it. However, even if I had included it, it still wouldn't work as mentioned before.... cough cough.

Also, inspired by default actions (link), another shortcut might be added to input mode:

The default values are mimicking all the hardcoded stuff, I do not want to change that as it would mean offtorque (why can't I tag him?) would need to redo his work with the upcoming revision of the guide, which, in my opinion is unnecessary at this point. Also, people might find that change off-putting, best left to every user to make any adjustments as they see fit. In my opinion that defaults.lua file should never be touched again ;)