Merge lp:~googol-deactivatedaccount/openlp/bug-900399 into lp:openlp

Proposed by Andreas Preikschat
Status: Superseded
Proposed branch: lp:~googol-deactivatedaccount/openlp/bug-900399
Merge into: lp:openlp
Diff against target: 194 lines (+24/-39)
6 files modified
openlp/core/lib/plugin.py (+11/-10)
openlp/core/lib/pluginmanager.py (+8/-24)
openlp/core/ui/mainwindow.py (+1/-1)
openlp/plugins/media/mediaplugin.py (+1/-1)
openlp/plugins/presentations/presentationplugin.py (+2/-2)
openlp/plugins/songs/forms/editsongform.py (+1/-1)
To merge this branch: bzr merge lp:~googol-deactivatedaccount/openlp/bug-900399
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Review via email: mp+84842@code.launchpad.net

This proposal has been superseded by a proposal from 2011-12-08.

Commit message

bzr commit -m "-Fixed media item recreation (which caused bug #900399)
- Doc/method clean up
- Replaced continue with break
- Replaced method with one-liner"

Description of the change

Hello,

1) Fixed media item recreation (which caused bug #900399)
2) Doc/method clean up
3) Replaced continue with break (after "continuing" the first the x times follow where we continue, so we can just break the first time instead)
4) Replaced method with one-liner

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) wrote :

131 is an incorrect removal as when a plugin is disabled the settings dialog will fail as the value ins not defined.
Renaming fields is also iffy in a feature freeze. They are not a bug.
Code simplification is just Ok.

review: Needs Fixing
1822. By Andreas Preikschat

set settings_tab to None if plugin is disabled

1823. By Andreas Preikschat

assign variables in methods instead of returning them

1824. By Andreas Preikschat

fixed plugins

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/lib/plugin.py'
2--- openlp/core/lib/plugin.py 2011-10-26 20:11:15 +0000
3+++ openlp/core/lib/plugin.py 2011-12-08 16:37:24 +0000
4@@ -91,8 +91,9 @@
5 ``checkPreConditions()``
6 Provides the Plugin with a handle to check if it can be loaded.
7
8- ``getMediaManagerItem()``
9- Returns an instance of MediaManagerItem to be used in the Media Manager.
10+ ``createMediaManagerItem()``
11+ Creates a new instance of MediaManagerItem to be used in the Media
12+ Manager.
13
14 ``addImportMenuItem(import_menu)``
15 Add an item to the Import menu.
16@@ -100,8 +101,8 @@
17 ``addExportMenuItem(export_menu)``
18 Add an item to the Export menu.
19
20- ``getSettingsTab()``
21- Returns an instance of SettingsTabItem to be used in the Settings
22+ ``createSettingsTab()``
23+ Creates a new instance of SettingsTabItem to be used in the Settings
24 dialog.
25
26 ``addToMenu(menubar)``
27@@ -178,7 +179,7 @@
28 Provides the Plugin with a handle to check if it can be loaded.
29 Failing Preconditions does not stop a settings Tab being created
30
31- Returns True or False.
32+ Returns ``True`` or ``False``.
33 """
34 return True
35
36@@ -210,10 +211,10 @@
37 """
38 return self.status == PluginStatus.Active
39
40- def getMediaManagerItem(self):
41+ def createMediaManagerItem(self):
42 """
43 Construct a MediaManagerItem object with all the buttons and things
44- you need, and return it for integration into openlp.org.
45+ you need, and return it for integration into OpenLP.
46 """
47 if self.media_item_class:
48 return self.media_item_class(self.mediadock.media_dock, self,
49@@ -247,10 +248,10 @@
50 """
51 pass
52
53- def getSettingsTab(self, parent):
54+ def createSettingsTab(self, parent):
55 """
56- Create a tab for the settings window to display the configurable
57- options for this plugin to the user.
58+ Create a tab for the settings window to display the configurable options
59+ for this plugin to the user.
60 """
61 if self.settings_tab_class:
62 return self.settings_tab_class(parent, self.name,
63
64=== modified file 'openlp/core/lib/pluginmanager.py'
65--- openlp/core/lib/pluginmanager.py 2011-12-03 12:51:40 +0000
66+++ openlp/core/lib/pluginmanager.py 2011-12-08 16:37:24 +0000
67@@ -90,7 +90,7 @@
68 thisdepth = len(path.split(os.sep))
69 if thisdepth - startdepth > 2:
70 # skip anything lower down
71- continue
72+ break
73 modulename = os.path.splitext(path)[0]
74 prefix = os.path.commonprefix([self.basepath, path])
75 # hack off the plugin base path
76@@ -113,7 +113,7 @@
77 plugin_objects.append(plugin)
78 except TypeError:
79 log.exception(u'Failed to load plugin %s', unicode(p))
80- plugins_list = sorted(plugin_objects, self.order_by_weight)
81+ plugins_list = sorted(plugin_objects, key=lambda plugin: plugin.weight)
82 for plugin in plugins_list:
83 if plugin.checkPreConditions():
84 log.debug(u'Plugin %s active', unicode(plugin.name))
85@@ -122,29 +122,13 @@
86 plugin.status = PluginStatus.Disabled
87 self.plugins.append(plugin)
88
89- def order_by_weight(self, x, y):
90- """
91- Sort two plugins and order them by their weight.
92-
93- ``x``
94- The first plugin.
95-
96- ``y``
97- The second plugin.
98- """
99- return cmp(x.weight, y.weight)
100-
101- def hook_media_manager(self, mediadock):
102- """
103- Loop through all the plugins. If a plugin has a valid media manager
104- item, add it to the media manager.
105-
106- ``mediatoolbox``
107- The Media Manager itself.
108+ def hook_media_manager(self):
109+ """
110+ Create the plugins' media manager items.
111 """
112 for plugin in self.plugins:
113 if plugin.status is not PluginStatus.Disabled:
114- plugin.mediaItem = plugin.getMediaManagerItem()
115+ plugin.mediaItem = plugin.createMediaManagerItem()
116
117 def hook_settings_tabs(self, settings_form=None):
118 """
119@@ -152,12 +136,12 @@
120 item, add it to the settings tab.
121 Tabs are set for all plugins not just Active ones
122
123- ``settingsform``
124+ ``settings_form``
125 Defaults to *None*. The settings form to add tabs to.
126 """
127 for plugin in self.plugins:
128 if plugin.status is not PluginStatus.Disabled:
129- plugin.settings_tab = plugin.getSettingsTab(settings_form)
130+ plugin.settings_tab = plugin.createSettingsTab(settings_form)
131 else:
132 plugin.settings_tab = None
133 settings_form.plugins = self.plugins
134
135=== modified file 'openlp/core/ui/mainwindow.py'
136--- openlp/core/ui/mainwindow.py 2011-12-05 21:40:56 +0000
137+++ openlp/core/ui/mainwindow.py 2011-12-08 16:37:24 +0000
138@@ -655,7 +655,7 @@
139 self.pluginManager.hook_settings_tabs(self.settingsForm)
140 # Find and insert media manager items
141 log.info(u'hook media')
142- self.pluginManager.hook_media_manager(self.mediaDockManager)
143+ self.pluginManager.hook_media_manager()
144 # Call the hook method to pull in import menus.
145 log.info(u'hook menus')
146 self.pluginManager.hook_import_menu(self.fileImportMenu)
147
148=== modified file 'openlp/plugins/media/mediaplugin.py'
149--- openlp/plugins/media/mediaplugin.py 2011-12-02 21:13:05 +0000
150+++ openlp/plugins/media/mediaplugin.py 2011-12-08 16:37:24 +0000
151@@ -52,7 +52,7 @@
152 for ext in self.video_extensions_list:
153 self.serviceManager.supportedSuffixes(ext[2:])
154
155- def getSettingsTab(self, parent):
156+ def createSettingsTab(self, parent):
157 """
158 Create the settings Tab
159 """
160
161=== modified file 'openlp/plugins/presentations/presentationplugin.py'
162--- openlp/plugins/presentations/presentationplugin.py 2011-10-03 20:12:57 +0000
163+++ openlp/plugins/presentations/presentationplugin.py 2011-12-08 16:37:24 +0000
164@@ -57,7 +57,7 @@
165 self.icon_path = u':/plugins/plugin_presentations.png'
166 self.icon = build_icon(self.icon_path)
167
168- def getSettingsTab(self, parent):
169+ def createSettingsTab(self, parent):
170 """
171 Create the settings Tab
172 """
173@@ -94,7 +94,7 @@
174 controller.kill()
175 Plugin.finalise(self)
176
177- def getMediaManagerItem(self):
178+ def createMediaManagerItem(self):
179 """
180 Create the Media Manager List
181 """
182
183=== modified file 'openlp/plugins/songs/forms/editsongform.py'
184--- openlp/plugins/songs/forms/editsongform.py 2011-12-02 20:17:57 +0000
185+++ openlp/plugins/songs/forms/editsongform.py 2011-12-08 16:37:24 +0000
186@@ -181,7 +181,7 @@
187 plugin.status == PluginStatus.Active:
188 self.audioAddFromMediaButton.setVisible(True)
189 self.mediaForm.populateFiles(
190- plugin.getMediaManagerItem().getList(MediaType.Audio))
191+ plugin.mediaItem.getList(MediaType.Audio))
192 break
193
194 def newSong(self):