Skip to content
Permalink
Browse files

Don't re-register an already registered action in QgsShortcutsManager

Avoids incorrect warnings about duplicate shortcuts on startup.

What's happening here is:
- on QGIS startup, plugins are loaded, adding their actions to
the interface via iface.registerMainWindowAction()
- after ALL plugins and qgis native menus and actions are created,
the shortcut manager registers ALL children from the main window.
This includes the actions and widgets created by plugins, which
have already been registered to the manager.
- There's no way to avoid this duplicate registration - we could
move the child shortcut registration to occur before plugin
initialization, but it's actually nice to have this "catch-all"
occur after plugins are loaded (so that plugins which don't
correctly register actions still have them included in the shortcut
manager). Similarly, plugins MUST use the registerMainWindowAction
call instead of just relying on the Qt QAction.setShortcut method
because otherwise the shortcuts manager is unaware of actions
created after QGIS load - e.g. enabling a plugin after startup.

So we avoid this by just refusing to re-register a shortcut
that the manager is already aware of... no more startup warnings!
  • Loading branch information
nyalldawson committed Jan 24, 2018
1 parent 63db1be commit c41b2dd11c787c13737883502f90662f9f86d16f
Showing with 8 additions and 0 deletions.
  1. +3 −0 src/gui/qgsshortcutsmanager.cpp
  2. +5 −0 tests/src/python/test_qgsshortcutsmanager.py
@@ -77,6 +77,9 @@ void QgsShortcutsManager::registerAllChildShortcuts( QObject *object, bool recur

bool QgsShortcutsManager::registerAction( QAction *action, const QString &defaultSequence )
{
if ( mActions.contains( action ) )
return false; // already registered

#ifdef QGISDEBUG
// if using a debug build, warn on duplicate actions
if ( actionByName( action->text() ) || shortcutByName( action->text() ) )
@@ -90,6 +90,11 @@ def testRegisterAction(self):
action2 = QAction('action2', None)
action2.setShortcut('y')
self.assertTrue(s.registerAction(action2, 'B'))
self.assertCountEqual(s.listActions(), [action1, action2])

# try re-registering an existing action - should fail, but leave action registered
self.assertFalse(s.registerAction(action2, 'B'))
self.assertCountEqual(s.listActions(), [action1, action2])

# actions should have been set to default sequences
self.assertEqual(action1.shortcut().toString(), 'A')

0 comments on commit c41b2dd

Please sign in to comment.
You can’t perform that action at this time.