Keyboard shortcuts change for 0.8.14 - impact on plugins?

[kiwidude]

One for Kovid I guess...

I see you have a shiny new preference for editing keyboard shortcuts. My question concerns what does it mean from a plugin perspective?

You seem to have some kind of discovery going on, so I see some of the shortcuts for "Find Duplicates" and "Goodreads", "Open with" etc appearing in the new preferences screen.

I am imagining how this works is that because my plugins are registering keyboard shortcuts, you are then able through Qt to do some kind of "currently hooked in shortcuts" to display in the new dialog.

However I forsee a number of potential issues/conflicts. For instance since this functionality did not exist previously, all my plugins have had to have their own ability to both define and persist a shortcut. In some cases these are relatively simply defined being static - e.g. next/previous result for find duplicates. In other cases they are truly dynamic - e.g. Search the Internet or Open With, where a user can assign any number of shortcuts to "rows" of data.

So I can see a conflict happening if a user were to attempt to change a shortcut in your new dialog (as my plugin would try to register at startup its "known" shortcut that it has persisted). And your screen would not be able to allow configuring shortcuts for plugins for functions not yet assigned a shortcut, since it doesn't know about them.

Got any ideas as to what this means for plugins with keyboard shortcuts?

[kovidgoyal]

I suggest removing the persistence of shortcuts from your plugins, use calibre's global functionality for that. Depending on exactly how you implement shortcuts in your plugins, the shortcut persistence in the plugin may lose or it may win.

General background on this change:

1) Any shortcut defined via the action_spec field in your plugin class is automatically managed by calibre. Do not persist these manually.

2) If you want to define shortcuts for more actions (for example for actions in the menu of the main action, use the convenience methods

create_action or create_menu_action

to register these with calibre automatically. All the builtin plugins have been migrated to use create_menu_action, so it should be easy to see how they are used.

3) If for any reason you dont want to use create_menu_action, use gui.keyboard.register_shortcut instead. See the register_shortcut method in gui2/keyboard.py

[kiwidude]

I figured you would have a cunning plan, you just saved me some trawling to discover what it was...

Hmmm. I guess just about every GUI plugin I have written will need an update then. In the meantime, provided users don't attempt to change the shortcuts for one of my plugins from your new screen there is no issue. So I can just push the plugins out over the next week or two hopefully as I figure out an approach. I will have to think about if/how to migrate users who have existing shortcuts configured as well.

I aways knew this "pain" would come eventually, like going to the dentist, you know it has to be done but you still don't look forward to it...

I may have more questions on this but I will wait until I actually find time to look into it properly. Thx.

[kovidgoyal]

I suggest waiting, this code is rather new and rather complex, so there are bound to be bugs. In the meantime if your manual persistence loses out, your users will just have to make the same customization globally in calibre.

[kiwidude]

Thx Kovid, waiting sounds good to me

[kiwidude]

Ok, so I'm just starting to look into this again. Firstly I think there is a minor bug - if you go to Preferences -> Keyboard, click on an existing shortcut, don't change anything (so radio button of default is set) and click Done, then it shows "Shortcuts: None". When in actual fact it is really still the default shortcut. To genuinely set a shortcut to none, you choose "Custom" and click Done - but visually you cannot see the difference of "None" until you close and come back into the dialog?

Anyhow, back to the main challenge - plugin shortcuts. I'm seeing different behaviour from the "InterfaceAction.create_action" versus "create_menu_action" functions. This could well be intentional but it would be good to hear your intentions with these?

Currently my plugins all call create_action(). Which means that they will only appear in the Preferences->Keyboard dialog if at the time I call create_action I am setting a shortcut value.

However as you mention above the Calibre plugins are calling create_menu_action(). Which has a differing behaviour - if no shortcut is specified they will still appear in your Keyboard dialog.

I know create_menu_action() is a new addition as part of these changes - I just want to figure out when I should be using which?

[kovidgoyal]

create_action is always called for action_spec, so it allows you to turn off shortcut registry by setting the shortcut to None. If you want to register and still not set a shortcut, use () instead of None.

As for best practice, use whatever feels comfortable for you. create_menu_action is a convenience method to ease the common task of create an action and adding it to a menu.

[kiwidude]

Ahhh - () is the magic trick, thanks. There is quite a bit of "different" looking code between the two methods so I wasn't sure if there were other intended differences. I have my own plugin wrapper function from previous so just figuring out the combination I want to use.

Another question - as I mentioned above all my plugins have had to include on their configuration dialogs a way for the user to customise shortcuts. Now the Calibre dialog exists, that offers a more powerful alternative.

So for a plugin that has a fixed number of menu actions (e.g. Find Duplicates), the question is whether I should rip out all the GUI configuration in my plugin for these. My inclination is to rip them out, and for those plugins leave all keyboard shortcut configuration via the Calibre dialog in Preferences->Plugins.

The only downside I see is that a user needs to go to two places to configure a plugin. There are some other options that "might" be possible, but it is the simplest approach.

The second part of this is for plugins that have a dynamic number of actions (plus sometimes some static ones as well). Open With, Search the Internet, View Manager & Reading List are all examples of this. The user can dynamically add more menu items in all these plugins and have keyboard shortcuts associated with them (sometimes multiple). Do you think it acceptable that I completely remove all shortcut configuration from the plugins, and leave it to the user to go into the completely separate Calibre dialog to associate shortcuts?

Again I am inclined to take the "easy" route and say all shortcuts are via Calibre's dialog. The pluses being consistency and the user gets instant feedback of conflicts and a more user friendly UI.

Any opinions from anyone as to what they would like the "standard" for Calibre to be (since with only a couple of exceptions like meme's Kindle plugin I have written darn near all the GUI plugins in the forum).

My final comment is that from an upgrade perspective, I have all but decided that users will lose any shortcuts they have configured, and must manually recreate them in the Calibre dialog. I think it will be way too complicated to try to migrate shortcuts stored in config files for existing users into the new way of doing things.

[kovidgoyal]

I'm good with you using the calibre UI for shortcut config. Note that the preferences widgets are designed to be embeddable, so you could embed the keyboard shortcuts widget in your config dialog. See for example show_config_widget() which embeds a preferences widget in its own QDialog. It should even be possible to highligh/expand the shortcuts for your particular plugin when doing so. There may be a few gotchas with doing this, but I'll be happy to help smooth them out

[kiwidude]

@Kovid - there is another problem with the Preferences->Keyboard dialog. Try customising one of the shortcuts at the very bottom of the list. The scrollbar does not scroll the expanded panel into view, but click on the scrollbar to scroll down then collapses the panel since it loses focus. So unless you click on one of the radio buttons first it is impossible to see the contents.

[kovidgoyal]

Originally Posted by kiwidude

@Kovid - there is another problem with the Preferences->Keyboard dialog. Try customising one of the shortcuts at the very bottom of the list. The scrollbar does not scroll the expanded panel into view, but click on the scrollbar to scroll down then collapses the panel since it loses focus. So unless you click on one of the radio buttons first it is impossible to see the contents.

Yeah, this is caused by a rather annoying bug in Qt, I wasn't able to find a workaround last time, but I'll give it another bash.

[kovidgoyal]

Fix committed.

[kiwidude]

The filtered view of the keyboard shortcuts in the widget to just those for the plugin sounds a nice compromise. It was one of the "other options" I had contemplated but wasn't sure how hard it would be - that you suggested it as well means I'll take a look. I could have a "Keyboard shortcuts..." button on my configuration dialogs that opens it up. So yes, any help you choose to volunteer for that is most appreciated

Another gotcha you may have an answer for. If I understand it correctly, the shortcut registration uses the text of the menu item to display in your widget. However I would "ideally" like there to be a way of providing a more descriptive text, that is only used in your widget?

The reason being for plugins like Reading List, the menu looks like this:
Add to list -> ListName1
Remove from list -> ListName1

The problem being that "ListName1" is the submenu action name that will have the keyboard shortcut. And a user can specify a shortcut for both Add and Remove actions. So right now I would have to change the sub menus to instead of just being "ListName1" to instead be "Remove from ListName1".

If instead via the create_menu_action etc I could supply a user friendly description for use in the keyboard widget, the user could see something like:
ListName1 (Add to list)
ListName1 (Remove from list)

Any thoughts? Or should I make all my menus more verbose

[kovidgoyal]

Added a shortcut_name parameter to create_menu_action()

On embedding: You wont be able to filter the list of shortcuts easily, but you could highlight and expand the node corresponding to your plugin.

[kiwidude]

Thanks for the list fix and shortcut_name. That answers my question which of create_action or create_menu_action I should use (unless you also add it to the other method)

Understood on not being able to filter, just expand/highlight which is fine.