Move some static methods out of their classes where it makes sense

openlp/core/lib/renderer.py

@@ -451,7 +451,86 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties):
         formatted.append(previous_raw)
         return formatted
 
-    @staticmethod
+    def _binary_chop(self, formatted, previous_html, previous_raw, html_list, raw_list, separator, line_end):
+        """
+        This implements the binary chop algorithm for faster rendering. This algorithm works line based (line by line)
+        and word based (word by word). It is assumed that this method is **only** called, when the lines/words to be
+        rendered do **not** fit as a whole.
+
+        :param formatted: The list to append any slides.
+        :param previous_html: The html text which is know to fit on a slide, but is not yet added to the list of
+        slides. (unicode string)
+        :param previous_raw: The raw text (with formatting tags) which is know to fit on a slide, but is not yet added
+        to the list of slides. (unicode string)
+        :param html_list: The elements which do not fit on a slide and needs to be processed using the binary chop.
+        The text contains html.
+        :param raw_list: The elements which do not fit on a slide and needs to be processed using the binary chop.
+        The elements can contain formatting tags.
+        :param separator: The separator for the elements. For lines this is ``'<br>'`` and for words this is ``' '``.
+        :param line_end: The text added after each "element line". Either ``' '`` or ``'<br>``. This is needed for
+         bibles.
+        """
+        smallest_index = 0
+        highest_index = len(html_list) - 1
+        index = highest_index // 2
+        while True:
+            if not self._text_fits_on_slide(previous_html + separator.join(html_list[:index + 1]).strip()):
+                # We know that it does not fit, so change/calculate the new index and highest_index accordingly.
+                highest_index = index
+                index = index - (index - smallest_index) // 2
+            else:
+                smallest_index = index
+                index = index + (highest_index - index) // 2
+            # We found the number of words which will fit.
+            if smallest_index == index or highest_index == index:
+                index = smallest_index
+                text = previous_raw.rstrip('<br>') + separator.join(raw_list[:index + 1])
+                text, raw_tags, html_tags = _get_start_tags(text)
+                formatted.append(text)
+                previous_html = ''
+                previous_raw = ''
+                # Stop here as the theme line count was requested.
+                if self.force_page:
+                    Registry().execute('theme_line_count', index + 1)
+                    break
+            else:
+                continue
+            # Check if the remaining elements fit on the slide.
+            if self._text_fits_on_slide(html_tags + separator.join(html_list[index + 1:]).strip()):
+                previous_html = html_tags + separator.join(html_list[index + 1:]).strip() + line_end
+                previous_raw = raw_tags + separator.join(raw_list[index + 1:]).strip() + line_end
+                break
+            else:
+                # The remaining elements do not fit, thus reset the indexes, create a new list and continue.
+                raw_list = raw_list[index + 1:]
+                raw_list[0] = raw_tags + raw_list[0]
+                html_list = html_list[index + 1:]
+                html_list[0] = html_tags + html_list[0]
+                smallest_index = 0
+                highest_index = len(html_list) - 1
+                index = highest_index // 2
+        return previous_html, previous_raw
+
+    def _text_fits_on_slide(self, text):
+        """
+        Checks if the given ``text`` fits on a slide. If it does ``True`` is returned, otherwise ``False``.
+
+        :param text:  The text to check. It may contain HTML tags.
+        """
+        self.web_frame.evaluateJavaScript('show_text("%s")' % text.replace('\\', '\\\\').replace('\"', '\\\"'))
+        return self.web_frame.contentsSize().height() <= self.empty_height
+
+
+def _words_split(line):
+    """
+    Split the slide up by word so can wrap better
+
+    :param line: Line to be split
+    """
+    # this parse we are to be wordy
+    line = line.replace('\n', ' ')
+    return line.split(' ')
+
 def _get_start_tags(raw_text):
     """
     Tests the given text for not closed formatting tags and returns a tuple consisting of three unicode strings::
@@ -488,82 +567,3 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties):
     html_tags = [tag[1] for tag in html_tags]
     return raw_text + ''.join(end_tags), ''.join(start_tags), ''.join(html_tags)
 
-    def _binary_chop(self, formatted, previous_html, previous_raw, html_list, raw_list, separator, line_end):
-        """
-        This implements the binary chop algorithm for faster rendering. This algorithm works line based (line by line)
-        and word based (word by word). It is assumed that this method is **only** called, when the lines/words to be
-        rendered do **not** fit as a whole.
-
-        :param formatted: The list to append any slides.
-        :param previous_html: The html text which is know to fit on a slide, but is not yet added to the list of
-        slides. (unicode string)
-        :param previous_raw: The raw text (with formatting tags) which is know to fit on a slide, but is not yet added
-        to the list of slides. (unicode string)
-        :param html_list: The elements which do not fit on a slide and needs to be processed using the binary chop.
-        The text contains html.
-        :param raw_list: The elements which do not fit on a slide and needs to be processed using the binary chop.
-        The elements can contain formatting tags.
-        :param separator: The separator for the elements. For lines this is ``'<br>'`` and for words this is ``' '``.
-        :param line_end: The text added after each "element line". Either ``' '`` or ``'<br>``. This is needed for
-         bibles.
-        """
-        smallest_index = 0
-        highest_index = len(html_list) - 1
-        index = highest_index // 2
-        while True:
-            if not self._text_fits_on_slide(previous_html + separator.join(html_list[:index + 1]).strip()):
-                # We know that it does not fit, so change/calculate the new index and highest_index accordingly.
-                highest_index = index
-                index = index - (index - smallest_index) // 2
-            else:
-                smallest_index = index
-                index = index + (highest_index - index) // 2
-            # We found the number of words which will fit.
-            if smallest_index == index or highest_index == index:
-                index = smallest_index
-                text = previous_raw.rstrip('<br>') + separator.join(raw_list[:index + 1])
-                text, raw_tags, html_tags = Renderer._get_start_tags(text)
-                formatted.append(text)
-                previous_html = ''
-                previous_raw = ''
-                # Stop here as the theme line count was requested.
-                if self.force_page:
-                    Registry().execute('theme_line_count', index + 1)
-                    break
-            else:
-                continue
-            # Check if the remaining elements fit on the slide.
-            if self._text_fits_on_slide(html_tags + separator.join(html_list[index + 1:]).strip()):
-                previous_html = html_tags + separator.join(html_list[index + 1:]).strip() + line_end
-                previous_raw = raw_tags + separator.join(raw_list[index + 1:]).strip() + line_end
-                break
-            else:
-                # The remaining elements do not fit, thus reset the indexes, create a new list and continue.
-                raw_list = raw_list[index + 1:]
-                raw_list[0] = raw_tags + raw_list[0]
-                html_list = html_list[index + 1:]
-                html_list[0] = html_tags + html_list[0]
-                smallest_index = 0
-                highest_index = len(html_list) - 1
-                index = highest_index // 2
-        return previous_html, previous_raw
-
-    def _text_fits_on_slide(self, text):
-        """
-        Checks if the given ``text`` fits on a slide. If it does ``True`` is returned, otherwise ``False``.
-
-        :param text:  The text to check. It may contain HTML tags.
-        """
-        self.web_frame.evaluateJavaScript('show_text("%s")' % text.replace('\\', '\\\\').replace('\"', '\\\"'))
-        return self.web_frame.contentsSize().height() <= self.empty_height
-
-    @staticmethod
-    def _words_split(line):
-        """
-        Split the slide up by word so can wrap better
-
-        :param line: Line to be split
-        """
-        # this parse we are to be wordy
-        line = line.replace('\n', ' ')
-        return line.split(' ')

openlp/core/ui/mainwindow.py

@@ -47,6 +47,7 @@ from openlp.core.utils import LanguageManager, add_actions, get_application_vers
 from openlp.core.utils.actions import ActionList, CategoryOrder
 from openlp.core.ui.firsttimeform import FirstTimeForm
 from openlp.core.ui.projector.manager import ProjectorManager
+from openlp.core.ui.printserviceform import PrintServiceForm
 
 log = logging.getLogger(__name__)
 
@@ -197,7 +198,7 @@ class Ui_MainWindow(object):
                                                triggers=self.service_manager_contents.save_file_as)
         self.print_service_order_item = create_action(main_window, 'printServiceItem', can_shortcuts=True,
                                                       category=UiStrings().File,
-                                                      triggers=self.service_manager_contents.print_service_order)
+                                                      triggers=lambda x: PrintServiceForm().exec())
         self.file_exit_item = create_action(main_window, 'fileExitItem', icon=':/system/system_exit.png',
                                             can_shortcuts=True,
                                             category=UiStrings().File, triggers=main_window.close)

openlp/core/ui/servicemanager.py

@@ -144,8 +144,8 @@ class Ui_ServiceManager(object):
         self.service_manager_list.customContextMenuRequested.connect(self.context_menu)
         self.service_manager_list.setObjectName('service_manager_list')
         # enable drop
-        self.service_manager_list.__class__.dragEnterEvent = Ui_ServiceManager.drag_enter_event
+        self.service_manager_list.__class__.dragEnterEvent = lambda x, event: event.accept()
-        self.service_manager_list.__class__.dragMoveEvent = Ui_ServiceManager.drag_enter_event
+        self.service_manager_list.__class__.dragMoveEvent = lambda x, event: event.accept()
         self.service_manager_list.__class__.dropEvent = self.drop_event
         self.layout.addWidget(self.service_manager_list)
         # Add the bottom toolbar
@@ -293,15 +293,6 @@ class Ui_ServiceManager(object):
         Registry().register_function('theme_update_global', self.theme_change)
         Registry().register_function('mediaitem_suffix_reset', self.reset_supported_suffixes)
 
-    @staticmethod
-    def drag_enter_event(event):
-        """
-        Accept Drag events
-
-        :param event: Handle of the event passed
-        """
-        event.accept()
-
 
 class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceManager, RegistryProperties):
     """
@@ -1586,7 +1577,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
                 if item is None:
                     end_pos = len(self.service_items)
                 else:
-                    end_pos = ServiceManager._get_parent_item_data(item) - 1
+                    end_pos = _get_parent_item_data(item) - 1
                 service_item = self.service_items[start_pos]
                 self.service_items.remove(service_item)
                 self.service_items.insert(end_pos, service_item)
@@ -1599,21 +1590,21 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
                     self.drop_position = len(self.service_items)
                 else:
                     # we are over something so lets investigate
-                    pos = ServiceManager._get_parent_item_data(item) - 1
+                    pos = _get_parent_item_data(item) - 1
                     service_item = self.service_items[pos]
                     if (plugin == service_item['service_item'].name and
                             service_item['service_item'].is_capable(ItemCapabilities.CanAppend)):
                         action = self.dnd_menu.exec(QtGui.QCursor.pos())
                         # New action required
                         if action == self.new_action:
-                            self.drop_position = ServiceManager._get_parent_item_data(item)
+                            self.drop_position = _get_parent_item_data(item)
                         # Append to existing action
                         if action == self.add_to_action:
-                            self.drop_position = ServiceManager._get_parent_item_data(item)
+                            self.drop_position = _get_parent_item_data(item)
                             item.setSelected(True)
                             replace = True
                     else:
-                        self.drop_position = ServiceManager._get_parent_item_data(item) - 1
+                        self.drop_position = _get_parent_item_data(item) - 1
                 Registry().execute('%s_add_service_item' % plugin, replace)
 
     def update_theme_list(self, theme_list):
@@ -1657,7 +1648,13 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
         self.service_items[item]['service_item'].update_theme(theme)
         self.regenerate_service_items(True)
 
-    @staticmethod
+    def get_drop_position(self):
+        """
+        Getter for drop_position. Used in: MediaManagerItem
+        """
+        return self.drop_position
+
+
 def _get_parent_item_data(item):
     """
     Finds and returns the parent item for any item
@@ -1670,16 +1667,3 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa
     else:
         return parent_item.data(0, QtCore.Qt.UserRole)
 
-    @staticmethod
-    def print_service_order(field=None):
-        """
-        Print a Service Order Sheet.
-        """
-        setting_dialog = PrintServiceForm()
-        setting_dialog.exec()
-
-    def get_drop_position(self):
-        """
-        Getter for drop_position. Used in: MediaManagerItem
-        """
-        return self.drop_position

openlp/plugins/songs/forms/songexportform.py

@@ -72,7 +72,7 @@ class SongExportForm(OpenLPWizard):
         """
         Song wizard specific signals.
         """
-        self.available_list_widget.itemActivated.connect(SongExportForm.on_item_activated)
+        self.available_list_widget.itemActivated.connect(_on_item_activated)
         self.search_line_edit.textEdited.connect(self.on_search_line_edit_changed)
         self.uncheck_button.clicked.connect(self.on_uncheck_button_clicked)
         self.check_button.clicked.connect(self.on_check_button_clicked)
@@ -172,7 +172,7 @@ class SongExportForm(OpenLPWizard):
             return True
         elif self.currentPage() == self.available_songs_page:
             items = [
-                item for item in SongExportForm._find_list_widget_items(self.available_list_widget) if item.checkState()
+                item for item in _find_list_widget_items(self.available_list_widget) if item.checkState()
             ]
             if not items:
                 critical_error_message_box(
@@ -241,7 +241,7 @@ class SongExportForm(OpenLPWizard):
         """
         songs = [
             song.data(QtCore.Qt.UserRole)
-            for song in SongExportForm._find_list_widget_items(self.selected_list_widget)
+            for song in _find_list_widget_items(self.selected_list_widget)
         ]
         exporter = OpenLyricsExport(self, songs, self.directory_line_edit.text())
         try:
@@ -255,30 +255,6 @@ class SongExportForm(OpenLPWizard):
             self.progress_label.setText(translate('SongsPlugin.SongExportForm', 'Your song export failed because this '
                                                   'error occurred: %s') % ose.strerror)
 
-    @staticmethod
-    def _find_list_widget_items(list_widget, text=''):
-        """
-        Returns a list of *QListWidgetItem*s of the ``list_widget``. Note, that hidden items are included.
-
-        :param list_widget: The widget to get all items from. (QListWidget)
-        :param text: The text to search for. (unicode string)
-        """
-        return [
-            item for item in list_widget.findItems(text, QtCore.Qt.MatchContains)
-        ]
-
-    @staticmethod
-    def on_item_activated(item):
-        """
-        Called, when an item in the *available_list_widget* has been triggered.
-        The item is check if it was not checked, whereas it is unchecked when it
-        was checked.
-
-        :param item:  The *QListWidgetItem* which was triggered.
-        """
-        item.setCheckState(
-            QtCore.Qt.Unchecked if item.checkState() else QtCore.Qt.Checked)
-
     def on_search_line_edit_changed(self, text):
         """
         The *search_line_edit*'s text has been changed. Update the list of
@@ -288,9 +264,9 @@ class SongExportForm(OpenLPWizard):
         :param text:  The text of the *search_line_edit*.
         """
         search_result = [
-            song for song in SongExportForm._find_list_widget_items(self.available_list_widget, text)
+            song for song in _find_list_widget_items(self.available_list_widget, text)
         ]
-        for item in SongExportForm._find_list_widget_items(self.available_list_widget):
+        for item in _find_list_widget_items(self.available_list_widget):
             item.setHidden(item not in search_result)
 
     def on_uncheck_button_clicked(self):
@@ -319,3 +295,26 @@ class SongExportForm(OpenLPWizard):
         self.get_folder(
             translate('SongsPlugin.ExportWizardForm', 'Select Destination Folder'),
             self.directory_line_edit, 'last directory export')
+
+
+def _find_list_widget_items(list_widget, text=''):
+    """
+    Returns a list of *QListWidgetItem*s of the ``list_widget``. Note, that hidden items are included.
+
+    :param list_widget: The widget to get all items from. (QListWidget)
+    :param text: The text to search for. (unicode string)
+    """
+    return [
+        item for item in list_widget.findItems(text, QtCore.Qt.MatchContains)
+    ]
+
+def _on_item_activated(item):
+    """
+    Called, when an item in the *available_list_widget* has been triggered.
+    The item is check if it was not checked, whereas it is unchecked when it
+    was checked.
+
+    :param item:  The *QListWidgetItem* which was triggered.
+    """
+    item.setCheckState(QtCore.Qt.Unchecked if item.checkState() else QtCore.Qt.Checked)
+

openlp/plugins/songs/lib/importers/foilpresenter.py

@@ -234,17 +234,6 @@ class FoilPresenter(object):
             clean_song(self.manager, song)
             self.manager.save_object(song)
 
-    @staticmethod
-    def _child(element):
-        """
-        This returns the text of an element as unicode string.
-
-        :param element: The element
-        """
-        if element is not None:
-            return str(element)
-        return ''
-
     def _process_authors(self, foilpresenterfolie, song):
         """
         Adds the authors specified in the XML to the song.
@@ -254,7 +243,7 @@ class FoilPresenter(object):
         """
         authors = []
         try:
-            copyright = FoilPresenter._child(foilpresenterfolie.copyright.text_)
+            copyright = _child(foilpresenterfolie.copyright.text_)
         except AttributeError:
             copyright = None
         if copyright:
@@ -347,7 +336,7 @@ class FoilPresenter(object):
         :param song: The song object.
         """
         try:
-            song.ccli_number = FoilPresenter._child(foilpresenterfolie.ccliid)
+            song.ccli_number = _child(foilpresenterfolie.ccliid)
         except AttributeError:
             song.ccli_number = ''
 
@@ -359,7 +348,7 @@ class FoilPresenter(object):
         :param song: The song object.
         """
         try:
-            song.comments = FoilPresenter._child(foilpresenterfolie.notiz)
+            song.comments = _child(foilpresenterfolie.notiz)
         except AttributeError:
             song.comments = ''
 
@@ -371,7 +360,7 @@ class FoilPresenter(object):
         :param song: The song object.
         """
         try:
-            song.copyright = FoilPresenter._child(foilpresenterfolie.copyright.text_)
+            song.copyright = _child(foilpresenterfolie.copyright.text_)
         except AttributeError:
             song.copyright = ''
 
@@ -397,19 +386,19 @@ class FoilPresenter(object):
             VerseType.tags[VerseType.PreChorus]: 1
         }
         if not hasattr(foilpresenterfolie.strophen, 'strophe'):
-            self.importer.log_error(FoilPresenter._child(foilpresenterfolie.titel),
+            self.importer.log_error(_child(foilpresenterfolie.titel),
                                     str(translate('SongsPlugin.FoilPresenterSongImport',
                                                   'Invalid Foilpresenter song file. No verses found.')))
             self.save_song = False
             return
         for strophe in foilpresenterfolie.strophen.strophe:
-            text = FoilPresenter._child(strophe.text_) if hasattr(strophe, 'text_') else ''
+            text = _child(strophe.text_) if hasattr(strophe, 'text_') else ''
-            verse_name = FoilPresenter._child(strophe.key)
+            verse_name = _child(strophe.key)
             children = strophe.getchildren()
             sortnr = False
             for child in children:
                 if child.tag == 'sortnr':
-                    verse_sortnr = FoilPresenter._child(strophe.sortnr)
+                    verse_sortnr = _child(strophe.sortnr)
                     sortnr = True
                 # In older Version there is no sortnr, but we need one
             if not sortnr:
@@ -485,7 +474,7 @@ class FoilPresenter(object):
         song.song_number = ''
         try:
             for bucheintrag in foilpresenterfolie.buch.bucheintrag:
-                book_name = FoilPresenter._child(bucheintrag.name)
+                book_name = _child(bucheintrag.name)
                 if book_name:
                     book = self.manager.get_object_filtered(Book, Book.name == book_name)
                     if book is None:
@@ -494,8 +483,8 @@ class FoilPresenter(object):
                         self.manager.save_object(book)
                     song.song_book_id = book.id
                     try:
-                        if FoilPresenter._child(bucheintrag.nummer):
+                        if _child(bucheintrag.nummer):
-                            song.song_number = FoilPresenter._child(bucheintrag.nummer)
+                            song.song_number = _child(bucheintrag.nummer)
                     except AttributeError:
                         pass
                     # We only support one song book, so take the first one.
@@ -513,13 +502,13 @@ class FoilPresenter(object):
         try:
             for title_string in foilpresenterfolie.titel.titelstring:
                 if not song.title:
-                    song.title = FoilPresenter._child(title_string)
+                    song.title = _child(title_string)
                     song.alternate_title = ''
                 else:
-                    song.alternate_title = FoilPresenter._child(title_string)
+                    song.alternate_title = _child(title_string)
         except AttributeError:
             # Use first line of first verse
-            first_line = FoilPresenter._child(foilpresenterfolie.strophen.strophe.text_)
+            first_line = _child(foilpresenterfolie.strophen.strophe.text_)
             song.title = first_line.split('\n')[0]
 
     def _process_topics(self, foilpresenterfolie, song):
@@ -531,7 +520,7 @@ class FoilPresenter(object):
         """
         try:
             for name in foilpresenterfolie.kategorien.name:
-                topic_text = FoilPresenter._child(name)
+                topic_text = _child(name)
                 if topic_text:
                     topic = self.manager.get_object_filtered(Topic, Topic.name == topic_text)
                     if topic is None:
@@ -541,3 +530,15 @@ class FoilPresenter(object):
                     song.topics.append(topic)
         except AttributeError:
             pass
+
+
+def _child(element):
+    """
+    This returns the text of an element as unicode string.
+
+    :param element: The element
+    """
+    if element is not None:
+        return str(element)
+    return ''
+

tests/functional/openlp_plugins/songs/test_foilpresenterimport.py

@@ -50,7 +50,7 @@ class TestFoilPresenter(TestCase):
     #   _process_topics
 
     def setUp(self):
-        self.child_patcher = patch('openlp.plugins.songs.lib.importers.foilpresenter.FoilPresenter._child')
+        self.child_patcher = patch('openlp.plugins.songs.lib.importers.foilpresenter._child')
         self.clean_song_patcher = patch('openlp.plugins.songs.lib.importers.foilpresenter.clean_song')
         self.objectify_patcher = patch('openlp.plugins.songs.lib.importers.foilpresenter.objectify')
         self.process_authors_patcher = \