From 38a2a3dee229310aaa42f10dbc1ba94b9ea4342e Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Tue, 6 Dec 2011 20:30:39 +0100
Subject: [PATCH 1/7] fixed bug 768495

---
 openlp/core/utils/actions.py | 59 +++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 5 deletions(-)

diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py
index 86ee69a48..525a18f37 100644
--- a/openlp/core/utils/actions.py
+++ b/openlp/core/utils/actions.py
@@ -188,6 +188,7 @@ class ActionList(object):
     actions or categories.
     """
     instance = None
+    shortcut_map = {}
 
     def __init__(self):
         self.categories = CategoryList()
@@ -226,17 +227,41 @@ class ActionList(object):
             self.categories[category].actions.append(action)
         else:
             self.categories[category].actions.add(action, weight)
-        if category is None:
-            # Stop here, as this action is not configurable.
-            return
         # Load the shortcut from the config.
         settings = QtCore.QSettings()
         settings.beginGroup(u'shortcuts')
         shortcuts = settings.value(action.objectName(),
             QtCore.QVariant(action.shortcuts())).toStringList()
+        settings.endGroup()
+        if not shortcuts:
+            action.setShortcuts([])
+            return
+        shortcuts = map(unicode, shortcuts)
+        # Check the alternate shortcut first, to avoid problems when the
+        # alternate shortcut becomes the primary shortcut after removing the
+        # (initial) primary shortcut due to confllicts.
+        if len(shortcuts) == 2:
+            existing_actions = ActionList.shortcut_map.get(shortcuts[1], [])
+            # Check for conflicts with other actions considering the shortcut
+            # context.
+            if self._shortcut_available(existing_actions, action):
+                actions = ActionList.shortcut_map.get(shortcuts[1], [])
+                actions.append(action)
+                ActionList.shortcut_map[shortcuts[1]] = actions
+            else:
+                shortcuts.remove(shortcuts[1])
+        # Check the primary shortcut.
+        existing_actions = ActionList.shortcut_map.get(shortcuts[0], [])
+        # Check for conflicts with other actions considering the shortcut
+        # context.
+        if self._shortcut_available(existing_actions, action):
+            actions = ActionList.shortcut_map.get(shortcuts[0], [])
+            actions.append(action)
+            ActionList.shortcut_map[shortcuts[0]] = actions
+        else:
+            shortcuts.remove(shortcuts[0])
         action.setShortcuts(
             [QtGui.QKeySequence(shortcut) for shortcut in shortcuts])
-        settings.endGroup()
 
     def remove_action(self, action, category=None):
         """
@@ -244,7 +269,7 @@ class ActionList(object):
         automatically removed.
 
         ``action``
-            The QAction object to be removed.
+            The ``QAction`` object to be removed.
 
         ``category``
             The name (unicode string) of the category, which contains the
@@ -279,6 +304,30 @@ class ActionList(object):
             return
         self.categories.add(name, weight)
 
+    def _shortcut_available(self, existing_actions, action):
+        """
+        Checks if the given ``action`` may use its assigned shortcut(s) or not.
+        Returns ``True`` or ``False.
+
+        ``existing_actions``
+            A list of actions which already use a particular shortcut.
+
+        ``action``
+            The action which wants to use a particular shortcut.
+        """
+        for existing_action in existing_actions:
+            if action is existing_action:
+                continue
+            if existing_action.parent() is action.parent():
+               return False
+            if existing_action.shortcutContext() in [QtCore.Qt.WindowShortcut,
+                QtCore.Qt.ApplicationShortcut]:
+                return False
+            if action.shortcutContext() in [QtCore.Qt.WindowShortcut,
+                QtCore.Qt.ApplicationShortcut]:
+                return False
+        return True
+
 
 class CategoryOrder(object):
     """

From 12547fad2a597a20df1a2fb97d84d0a587ce239e Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Wed, 7 Dec 2011 21:25:12 +0100
Subject: [PATCH 2/7] doc clean up, removed method with a one liner, fixed
 media item recreation bug

---
 openlp/core/lib/plugin.py                     | 21 ++++++-------
 openlp/core/lib/pluginmanager.py              | 30 ++++---------------
 openlp/core/ui/mainwindow.py                  |  2 +-
 openlp/plugins/media/mediaplugin.py           |  2 +-
 .../presentations/presentationplugin.py       |  4 +--
 openlp/plugins/songs/forms/editsongform.py    |  2 +-
 6 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/openlp/core/lib/plugin.py b/openlp/core/lib/plugin.py
index 76e54ef8d..92ac0be64 100644
--- a/openlp/core/lib/plugin.py
+++ b/openlp/core/lib/plugin.py
@@ -91,8 +91,9 @@ class Plugin(QtCore.QObject):
     ``checkPreConditions()``
         Provides the Plugin with a handle to check if it can be loaded.
 
-    ``getMediaManagerItem()``
-        Returns an instance of MediaManagerItem to be used in the Media Manager.
+    ``createMediaManagerItem()``
+        Creates a new instance of MediaManagerItem to be used in the Media
+        Manager.
 
     ``addImportMenuItem(import_menu)``
         Add an item to the Import menu.
@@ -100,8 +101,8 @@ class Plugin(QtCore.QObject):
     ``addExportMenuItem(export_menu)``
         Add an item to the Export menu.
 
-    ``getSettingsTab()``
-        Returns an instance of SettingsTabItem to be used in the Settings
+    ``createSettingsTab()``
+        Creates a new instance of SettingsTabItem to be used in the Settings
         dialog.
 
     ``addToMenu(menubar)``
@@ -178,7 +179,7 @@ class Plugin(QtCore.QObject):
         Provides the Plugin with a handle to check if it can be loaded.
         Failing Preconditions does not stop a settings Tab being created
 
-        Returns True or False.
+        Returns ``True`` or ``False``.
         """
         return True
 
@@ -210,10 +211,10 @@ class Plugin(QtCore.QObject):
         """
         return self.status == PluginStatus.Active
 
-    def getMediaManagerItem(self):
+    def createMediaManagerItem(self):
         """
         Construct a MediaManagerItem object with all the buttons and things
-        you need, and return it for integration into openlp.org.
+        you need, and return it for integration into OpenLP.
         """
         if self.media_item_class:
             return self.media_item_class(self.mediadock.media_dock, self,
@@ -247,10 +248,10 @@ class Plugin(QtCore.QObject):
         """
         pass
 
-    def getSettingsTab(self, parent):
+    def createSettingsTab(self, parent):
         """
-        Create a tab for the settings window to display the configurable
-        options for this plugin to the user.
+        Create a tab for the settings window to display the configurable options
+        for this plugin to the user.
         """
         if self.settings_tab_class:
             return self.settings_tab_class(parent, self.name,
diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py
index db5d9b60e..e29176386 100644
--- a/openlp/core/lib/pluginmanager.py
+++ b/openlp/core/lib/pluginmanager.py
@@ -113,7 +113,7 @@ class PluginManager(object):
                 plugin_objects.append(plugin)
             except TypeError:
                 log.exception(u'Failed to load plugin %s', unicode(p))
-        plugins_list = sorted(plugin_objects, self.order_by_weight)
+        plugins_list = sorted(plugin_objects, key=lambda plugin: plugin.weight)
         for plugin in plugins_list:
             if plugin.checkPreConditions():
                 log.debug(u'Plugin %s active', unicode(plugin.name))
@@ -122,29 +122,13 @@ class PluginManager(object):
                 plugin.status = PluginStatus.Disabled
             self.plugins.append(plugin)
 
-    def order_by_weight(self, x, y):
+    def hook_media_manager(self):
         """
-        Sort two plugins and order them by their weight.
-
-        ``x``
-            The first plugin.
-
-        ``y``
-            The second plugin.
-        """
-        return cmp(x.weight, y.weight)
-
-    def hook_media_manager(self, mediadock):
-        """
-        Loop through all the plugins. If a plugin has a valid media manager
-        item, add it to the media manager.
-
-        ``mediatoolbox``
-            The Media Manager itself.
+        Create the plugins' media manager items.
         """
         for plugin in self.plugins:
             if plugin.status is not PluginStatus.Disabled:
-                plugin.mediaItem = plugin.getMediaManagerItem()
+                plugin.mediaItem = plugin.createMediaManagerItem()
 
     def hook_settings_tabs(self, settings_form=None):
         """
@@ -152,14 +136,12 @@ class PluginManager(object):
         item, add it to the settings tab.
         Tabs are set for all plugins not just Active ones
 
-        ``settingsform``
+        ``settings_form``
             Defaults to *None*. The settings form to add tabs to.
         """
         for plugin in self.plugins:
             if plugin.status is not PluginStatus.Disabled:
-                plugin.settings_tab = plugin.getSettingsTab(settings_form)
-            else:
-                plugin.settings_tab = None
+                plugin.settings_tab = plugin.createSettingsTab(settings_form)
         settings_form.plugins = self.plugins
 
     def hook_import_menu(self, import_menu):
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index 9d546629b..be04455f6 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -655,7 +655,7 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow):
         self.pluginManager.hook_settings_tabs(self.settingsForm)
         # Find and insert media manager items
         log.info(u'hook media')
-        self.pluginManager.hook_media_manager(self.mediaDockManager)
+        self.pluginManager.hook_media_manager()
         # Call the hook method to pull in import menus.
         log.info(u'hook menus')
         self.pluginManager.hook_import_menu(self.fileImportMenu)
diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py
index 97f749689..41965d612 100644
--- a/openlp/plugins/media/mediaplugin.py
+++ b/openlp/plugins/media/mediaplugin.py
@@ -52,7 +52,7 @@ class MediaPlugin(Plugin):
         for ext in self.video_extensions_list:
             self.serviceManager.supportedSuffixes(ext[2:])
 
-    def getSettingsTab(self, parent):
+    def createSettingsTab(self, parent):
         """
         Create the settings Tab
         """
diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py
index 643ad14ad..7baf659e0 100644
--- a/openlp/plugins/presentations/presentationplugin.py
+++ b/openlp/plugins/presentations/presentationplugin.py
@@ -57,7 +57,7 @@ class PresentationPlugin(Plugin):
         self.icon_path = u':/plugins/plugin_presentations.png'
         self.icon = build_icon(self.icon_path)
 
-    def getSettingsTab(self, parent):
+    def createSettingsTab(self, parent):
         """
         Create the settings Tab
         """
@@ -94,7 +94,7 @@ class PresentationPlugin(Plugin):
                 controller.kill()
         Plugin.finalise(self)
 
-    def getMediaManagerItem(self):
+    def createMediaManagerItem(self):
         """
         Create the Media Manager List
         """
diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py
index c1566a639..776c3c88b 100644
--- a/openlp/plugins/songs/forms/editsongform.py
+++ b/openlp/plugins/songs/forms/editsongform.py
@@ -181,7 +181,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog):
                 plugin.status == PluginStatus.Active:
                 self.audioAddFromMediaButton.setVisible(True)
                 self.mediaForm.populateFiles(
-                    plugin.getMediaManagerItem().getList(MediaType.Audio))
+                    plugin.mediaItem.getList(MediaType.Audio))
                 break
 
     def newSong(self):

From 32f2dcac59f67adba301a4e018023b14f9ed01b9 Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Wed, 7 Dec 2011 21:28:26 +0100
Subject: [PATCH 3/7] reverted change

---
 openlp/core/utils/actions.py | 59 +++---------------------------------
 1 file changed, 5 insertions(+), 54 deletions(-)

diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py
index 525a18f37..86ee69a48 100644
--- a/openlp/core/utils/actions.py
+++ b/openlp/core/utils/actions.py
@@ -188,7 +188,6 @@ class ActionList(object):
     actions or categories.
     """
     instance = None
-    shortcut_map = {}
 
     def __init__(self):
         self.categories = CategoryList()
@@ -227,41 +226,17 @@ class ActionList(object):
             self.categories[category].actions.append(action)
         else:
             self.categories[category].actions.add(action, weight)
+        if category is None:
+            # Stop here, as this action is not configurable.
+            return
         # Load the shortcut from the config.
         settings = QtCore.QSettings()
         settings.beginGroup(u'shortcuts')
         shortcuts = settings.value(action.objectName(),
             QtCore.QVariant(action.shortcuts())).toStringList()
-        settings.endGroup()
-        if not shortcuts:
-            action.setShortcuts([])
-            return
-        shortcuts = map(unicode, shortcuts)
-        # Check the alternate shortcut first, to avoid problems when the
-        # alternate shortcut becomes the primary shortcut after removing the
-        # (initial) primary shortcut due to confllicts.
-        if len(shortcuts) == 2:
-            existing_actions = ActionList.shortcut_map.get(shortcuts[1], [])
-            # Check for conflicts with other actions considering the shortcut
-            # context.
-            if self._shortcut_available(existing_actions, action):
-                actions = ActionList.shortcut_map.get(shortcuts[1], [])
-                actions.append(action)
-                ActionList.shortcut_map[shortcuts[1]] = actions
-            else:
-                shortcuts.remove(shortcuts[1])
-        # Check the primary shortcut.
-        existing_actions = ActionList.shortcut_map.get(shortcuts[0], [])
-        # Check for conflicts with other actions considering the shortcut
-        # context.
-        if self._shortcut_available(existing_actions, action):
-            actions = ActionList.shortcut_map.get(shortcuts[0], [])
-            actions.append(action)
-            ActionList.shortcut_map[shortcuts[0]] = actions
-        else:
-            shortcuts.remove(shortcuts[0])
         action.setShortcuts(
             [QtGui.QKeySequence(shortcut) for shortcut in shortcuts])
+        settings.endGroup()
 
     def remove_action(self, action, category=None):
         """
@@ -269,7 +244,7 @@ class ActionList(object):
         automatically removed.
 
         ``action``
-            The ``QAction`` object to be removed.
+            The QAction object to be removed.
 
         ``category``
             The name (unicode string) of the category, which contains the
@@ -304,30 +279,6 @@ class ActionList(object):
             return
         self.categories.add(name, weight)
 
-    def _shortcut_available(self, existing_actions, action):
-        """
-        Checks if the given ``action`` may use its assigned shortcut(s) or not.
-        Returns ``True`` or ``False.
-
-        ``existing_actions``
-            A list of actions which already use a particular shortcut.
-
-        ``action``
-            The action which wants to use a particular shortcut.
-        """
-        for existing_action in existing_actions:
-            if action is existing_action:
-                continue
-            if existing_action.parent() is action.parent():
-               return False
-            if existing_action.shortcutContext() in [QtCore.Qt.WindowShortcut,
-                QtCore.Qt.ApplicationShortcut]:
-                return False
-            if action.shortcutContext() in [QtCore.Qt.WindowShortcut,
-                QtCore.Qt.ApplicationShortcut]:
-                return False
-        return True
-
 
 class CategoryOrder(object):
     """

From 66c183a6b8ee84f63de92a83680887a85cb4e699 Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Wed, 7 Dec 2011 21:41:19 +0100
Subject: [PATCH 4/7] break instead of continue

---
 openlp/core/lib/pluginmanager.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py
index e29176386..b124dce9e 100644
--- a/openlp/core/lib/pluginmanager.py
+++ b/openlp/core/lib/pluginmanager.py
@@ -90,7 +90,7 @@ class PluginManager(object):
                     thisdepth = len(path.split(os.sep))
                     if thisdepth - startdepth > 2:
                         # skip anything lower down
-                        continue
+                        break
                     modulename = os.path.splitext(path)[0]
                     prefix = os.path.commonprefix([self.basepath, path])
                     # hack off the plugin base path

From 4f9a80196696cf0ffc10b54976077784add24dfa Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Thu, 8 Dec 2011 17:34:18 +0100
Subject: [PATCH 5/7] set settings_tab to None if plugin is disabled

---
 openlp/core/lib/pluginmanager.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py
index b124dce9e..6e259dc2a 100644
--- a/openlp/core/lib/pluginmanager.py
+++ b/openlp/core/lib/pluginmanager.py
@@ -142,6 +142,8 @@ class PluginManager(object):
         for plugin in self.plugins:
             if plugin.status is not PluginStatus.Disabled:
                 plugin.settings_tab = plugin.createSettingsTab(settings_form)
+            else:
+                plugin.settings_tab = None
         settings_form.plugins = self.plugins
 
     def hook_import_menu(self, import_menu):

From 95aedfa5d966ace85ea4aa3dc61c4477afe1b246 Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Thu, 8 Dec 2011 18:45:33 +0100
Subject: [PATCH 6/7] assign variables in methods instead of returning them

---
 openlp/core/lib/plugin.py        | 12 +++++-------
 openlp/core/lib/pluginmanager.py |  8 +++-----
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/openlp/core/lib/plugin.py b/openlp/core/lib/plugin.py
index 92ac0be64..f9e7889a4 100644
--- a/openlp/core/lib/plugin.py
+++ b/openlp/core/lib/plugin.py
@@ -157,10 +157,10 @@ class Plugin(QtCore.QObject):
         self.icon = None
         self.media_item_class = media_item_class
         self.settings_tab_class = settings_tab_class
+        self.settings_tab = None
+        self.mediaItem = None
         self.weight = 0
         self.status = PluginStatus.Inactive
-        # Set up logging
-        self.log = logging.getLogger(self.name)
         self.previewController = plugin_helpers[u'preview']
         self.liveController = plugin_helpers[u'live']
         self.renderer = plugin_helpers[u'renderer']
@@ -217,9 +217,8 @@ class Plugin(QtCore.QObject):
         you need, and return it for integration into OpenLP.
         """
         if self.media_item_class:
-            return self.media_item_class(self.mediadock.media_dock, self,
-                self.icon)
-        return None
+            self.mediaItem = self.media_item_class(self.mediadock.media_dock,
+                self, self.icon)
 
     def addImportMenuItem(self, importMenu):
         """
@@ -254,10 +253,9 @@ class Plugin(QtCore.QObject):
         for this plugin to the user.
         """
         if self.settings_tab_class:
-            return self.settings_tab_class(parent, self.name,
+            self.settings_tab  = self.settings_tab_class(parent, self.name,
                 self.getString(StringContent.VisibleName)[u'title'],
                 self.icon_path)
-        return None
 
     def addToMenu(self, menubar):
         """
diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py
index 6e259dc2a..d63a37dde 100644
--- a/openlp/core/lib/pluginmanager.py
+++ b/openlp/core/lib/pluginmanager.py
@@ -128,7 +128,7 @@ class PluginManager(object):
         """
         for plugin in self.plugins:
             if plugin.status is not PluginStatus.Disabled:
-                plugin.mediaItem = plugin.createMediaManagerItem()
+                plugin.createMediaManagerItem()
 
     def hook_settings_tabs(self, settings_form=None):
         """
@@ -141,9 +141,7 @@ class PluginManager(object):
         """
         for plugin in self.plugins:
             if plugin.status is not PluginStatus.Disabled:
-                plugin.settings_tab = plugin.createSettingsTab(settings_form)
-            else:
-                plugin.settings_tab = None
+                plugin.createSettingsTab(settings_form)
         settings_form.plugins = self.plugins
 
     def hook_import_menu(self, import_menu):
@@ -209,7 +207,7 @@ class PluginManager(object):
 
     def get_plugin_by_name(self, name):
         """
-        Return the plugin which has a name with value ``name``
+        Return the plugin which has a name with value ``name``.
         """
         for plugin in self.plugins:
             if plugin.name == name:

From fe8f5fb963eb5b0edd8f0ee38a2221ab17d46296 Mon Sep 17 00:00:00 2001
From: Andreas Preikschat <googol@lavabit.com>
Date: Thu, 8 Dec 2011 18:55:12 +0100
Subject: [PATCH 7/7] fixed plugins

---
 openlp/core/lib/plugin.py                          | 2 +-
 openlp/plugins/media/mediaplugin.py                | 2 +-
 openlp/plugins/presentations/presentationplugin.py | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/openlp/core/lib/plugin.py b/openlp/core/lib/plugin.py
index f9e7889a4..624ce2083 100644
--- a/openlp/core/lib/plugin.py
+++ b/openlp/core/lib/plugin.py
@@ -253,7 +253,7 @@ class Plugin(QtCore.QObject):
         for this plugin to the user.
         """
         if self.settings_tab_class:
-            self.settings_tab  = self.settings_tab_class(parent, self.name,
+            self.settings_tab = self.settings_tab_class(parent, self.name,
                 self.getString(StringContent.VisibleName)[u'title'],
                 self.icon_path)
 
diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py
index 41965d612..2825d899e 100644
--- a/openlp/plugins/media/mediaplugin.py
+++ b/openlp/plugins/media/mediaplugin.py
@@ -57,7 +57,7 @@ class MediaPlugin(Plugin):
         Create the settings Tab
         """
         visible_name = self.getString(StringContent.VisibleName)
-        return MediaTab(parent, self.name, visible_name[u'title'],
+        self.settings_tab = MediaTab(parent, self.name, visible_name[u'title'],
             self.mediaController.mediaPlayers, self.icon_path)
 
     def about(self):
diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py
index 7baf659e0..e35659638 100644
--- a/openlp/plugins/presentations/presentationplugin.py
+++ b/openlp/plugins/presentations/presentationplugin.py
@@ -62,8 +62,8 @@ class PresentationPlugin(Plugin):
         Create the settings Tab
         """
         visible_name = self.getString(StringContent.VisibleName)
-        return PresentationTab(parent, self.name, visible_name[u'title'],
-            self.controllers, self.icon_path)
+        self.settings_tab = PresentationTab(parent, self.name,
+            visible_name[u'title'], self.controllers, self.icon_path)
 
     def initialise(self):
         """
@@ -98,7 +98,7 @@ class PresentationPlugin(Plugin):
         """
         Create the Media Manager List
         """
-        return PresentationMediaItem(
+        self.mediaItem = PresentationMediaItem(
             self.mediadock.media_dock, self, self.icon, self.controllers)
 
     def registerControllers(self, controller):