openlp-core team mailing list archive

[Andreas Preikschat <andreaspreikschat@xxxxxxxxxx>]

Andreas Preikschat has proposed merging lp:~googol-hush/openlp/tweaks into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~googol-hush/openlp/tweaks/+merge/58907

Hello,

I fixed two things:

- when assigning an alternate shortcut to an action without shortcuts the alternate shortcuts automatically becomes the primary shortcut (which is correct), but the alternate button would still show the key sequence and the primary wouldn't (which is wrong).

- it was possible to assign the same shortcut to two different actions, when you used the "clear" button:
1) Change "Service Print" shortuct to "P"
2) Change "New" shortcut to "CTRL+P"
3) Select "Serivce Print" and press the clear button
-- 
https://code.launchpad.net/~googol-hush/openlp/tweaks/+merge/58907
Your team OpenLP Core is requested to review the proposed merge of lp:~googol-hush/openlp/tweaks into lp:openlp.

=== modified file 'openlp/core/ui/shortcutlistform.py'
--- openlp/core/ui/shortcutlistform.py	2011-04-09 18:14:51 +0000
+++ openlp/core/ui/shortcutlistform.py	2011-04-24 15:49:23 +0000
@@ -29,6 +29,7 @@
 
 from PyQt4 import QtCore, QtGui
 
+from openlp.core.lib import Receiver
 from openlp.core.utils import translate
 from openlp.core.utils.actions import ActionList
 from shortcutlistdialog import Ui_ShortcutListDialog
@@ -95,43 +96,7 @@
             QtCore.Qt.ShiftModifier:
             key_string = u'Shift+' + key_string
         key_sequence = QtGui.QKeySequence(key_string)
-        # The action we are attempting to change.
-        changing_action = self._currentItemAction()
-        shortcut_valid = True
-        for category in self.action_list.categories:
-            for action in category.actions:
-                shortcuts = self._actionShortcuts(action)
-                if key_sequence not in shortcuts:
-                    continue
-                if action is changing_action:
-                    if self.primaryPushButton.isChecked() and \
-                        shortcuts.index(key_sequence) == 0:
-                        continue
-                    if self.alternatePushButton.isChecked() and \
-                        shortcuts.index(key_sequence) == 1:
-                        continue
-                # Have the same parent, thus they cannot have the same shortcut.
-                if action.parent() is changing_action.parent():
-                    shortcut_valid = False
-                # The new shortcut is already assigned, but if both shortcuts
-                # are only valid in a different widget the new shortcut is
-                # vaild, because they will not interfere.
-                if action.shortcutContext() in [QtCore.Qt.WindowShortcut,
-                    QtCore.Qt.ApplicationShortcut]:
-                    shortcut_valid = False
-                if changing_action.shortcutContext() in \
-                    [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
-                    shortcut_valid = False
-        if not shortcut_valid:
-            QtGui.QMessageBox.warning(self,
-                translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'),
-                unicode(translate('OpenLP.ShortcutListDialog', 'The shortcut '
-                '"%s" is already assigned to another action, please '
-                'use a different shortcut.')) % key_sequence.toString(),
-                QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok),
-                QtGui.QMessageBox.Ok
-            )
-        else:
+        if self._validiate_shortcut(self._currentItemAction(), key_sequence):
             if self.primaryPushButton.isChecked():
                 self._adjustButton(self.primaryPushButton,
                     False, text=key_sequence.toString())
@@ -227,6 +192,12 @@
         new_shortcuts.append(
             QtGui.QKeySequence(self.alternatePushButton.text()))
         self.changedActions[action] = new_shortcuts
+        if not self.primaryPushButton.text():
+            # When we do not have a primary shortcut, the just entered alternate
+            # shortcut will automatically become the primary shortcut. That is
+            # why we have to adjust the primary button's text. 
+            self.primaryPushButton.setText(self.alternatePushButton.text())
+            self.alternatePushButton.setText(u'')
         self.refreshShortcutList()
 
     def onItemDoubleClicked(self, item, column):
@@ -374,6 +345,16 @@
         new_shortcuts = []
         if len(action.defaultShortcuts) != 0:
             new_shortcuts.append(action.defaultShortcuts[0])
+            # We have to check if the primary default shortcut is available. But
+            # we only have to check, if the action has a default primary
+            # shortcut (an "empty" shortcut is always valid and if the action
+            # does not have a default primary shortcut, then the alternative
+            # shortcut (not the default one) will become primary shortcut, thus
+            # the check will assume that an action were going to have the same
+            # shortcut twice.
+            if not self._validiate_shortcut(action, new_shortcuts[0]) and \
+                new_shortcuts[0] != shortcuts[0]:
+                return
         if len(shortcuts) == 2:
             new_shortcuts.append(shortcuts[1])
         self.changedActions[action] = new_shortcuts
@@ -394,10 +375,60 @@
             new_shortcuts.append(shortcuts[0])
         if len(action.defaultShortcuts) == 2:
             new_shortcuts.append(action.defaultShortcuts[1])
+        if len(new_shortcuts) == 2:
+            if not self._validiate_shortcut(action, new_shortcuts[1]):
+                return
         self.changedActions[action] = new_shortcuts
         self.refreshShortcutList()
         self.onCurrentItemChanged(self.treeWidget.currentItem())
 
+    def _validiate_shortcut(self, changing_action, key_sequence):
+        """
+        Checks if the given ``changing_action `` can use the given
+        ``key_sequence``. Returns ``True`` if the ``key_sequence`` can be used
+        by the action, otherwise displays a dialog and returns ``False``.
+
+        ``changing_action``
+            The action which wants to use the ``key_sequence``.
+
+        ``key_sequence``
+            The key sequence which the action want so use.
+        """
+        is_valid = True
+        for category in self.action_list.categories:
+            for action in category.actions:
+                shortcuts = self._actionShortcuts(action)
+                if key_sequence not in shortcuts:
+                    continue
+                if action is changing_action:
+                    if self.primaryPushButton.isChecked() and \
+                        shortcuts.index(key_sequence) == 0:
+                        continue
+                    if self.alternatePushButton.isChecked() and \
+                        shortcuts.index(key_sequence) == 1:
+                        continue
+                # Have the same parent, thus they cannot have the same shortcut.
+                if action.parent() is changing_action.parent():
+                    is_valid = False
+                # The new shortcut is already assigned, but if both shortcuts
+                # are only valid in a different widget the new shortcut is
+                # vaild, because they will not interfere.
+                if action.shortcutContext() in [QtCore.Qt.WindowShortcut,
+                    QtCore.Qt.ApplicationShortcut]:
+                    is_valid = False
+                if changing_action.shortcutContext() in \
+                    [QtCore.Qt.WindowShortcut, QtCore.Qt.ApplicationShortcut]:
+                    is_valid = False
+        if not is_valid:
+            Receiver.send_message(u'openlp_warning_message', {
+                u'title': translate('OpenLP.ShortcutListDialog',
+                'Duplicate Shortcut'),
+                u'message': unicode(translate('OpenLP.ShortcutListDialog',
+                'The shortcut "%s" is already assigned to another action, '
+                'please use a different shortcut.')) % key_sequence.toString()
+            })
+        return is_valid
+
     def _actionShortcuts(self, action):
         """
         This returns the shortcuts for the given ``action``, which also includes