Skip to content

[Hotkeys] refactor hotkey list and update callbacks#13219

Merged
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:hotkeys-ref
Feb 7, 2025
Merged

[Hotkeys] refactor hotkey list and update callbacks#13219
Frenzie merged 3 commits into
koreader:masterfrom
Commodore64user:hotkeys-ref

Conversation

@Commodore64user

@Commodore64user Commodore64user commented Feb 6, 2025

Copy link
Copy Markdown
Member

what's new

  • Renamed FFIUtil to ffiUtil to maintain a consistent naming convention across the code base.
  • Refactored the hotkeys list generation by defining a base keys table and iterating over it to create the hotkeys list dynamically. This change simplifies the code and makes it more maintainable.
  • Added no_refresh_on_check property to menu items to prevent unnecessary refreshes when checking the items.
  • Updated the callback functions in the hotkeys menu to include a touchmenu_instance parameter and handle the removal of actions more gracefully. This ensures the menu items are updated correctly after changes.

bug report

there is a problem with the callbacks introduced (in the gestures plugin) in #13078, I believe the solution is doing exactly what I am doing here (the if/else bit), but await instructions to see if I should do it here.


This change is Reviewable

Comment thread plugins/hotkeys.koplugin/main.lua
Comment thread plugins/hotkeys.koplugin/main.lua
@hius07

hius07 commented Feb 7, 2025

Copy link
Copy Markdown
Member

That doesn't meet our general style guide that functions take the form of doRemove() but I didn't realize it was copied from existing code either.

Except snake_cased_callbacks.

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for comments that just repeat what's on the line right below.

Comment thread plugins/hotkeys.koplugin/main.lua Outdated
separator = true,
checked_func = function()
return self.hotkeys[hotkey] == nil
-- Return true if no hotkey exists, or if the hotkey exists but has no dispatcher settings

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-- Return true if no hotkey exists, or if the hotkey exists but has no dispatcher settings

Comment thread plugins/hotkeys.koplugin/main.lua Outdated
end
if self.hotkeys[hotkey] and next(self.hotkeys[hotkey]) then
Dispatcher.removeActions(self.hotkeys[hotkey], do_remove)
else -- If no actions are selected, just update the defaults

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else -- If no actions are selected, just update the defaults

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you are suggesting I remove the comment, not the else

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because if you are suggesting I remove the else I encourage you to try it on you device and see what happens without it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, comments that just say what's happening are rarely useful. :-)

@Frenzie Frenzie added this to the 2025.02 milestone Feb 7, 2025
@Frenzie Frenzie merged commit 8e886f5 into koreader:master Feb 7, 2025
@Commodore64user Commodore64user deleted the hotkeys-ref branch February 7, 2025 13:48
@Commodore64user

Copy link
Copy Markdown
Member Author

just to be very clear one more time, there are two bugs, one is NT-specific described here #13078 (comment), and the other with Gestures not allowing you to select "Nothing" or "Pass trough" or "defaults" under certain circumstances.

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants