From 5bc13e45e3869e96cd8909482e3d494f6a1e3506 Mon Sep 17 00:00:00 2001 From: Simon Hanna Date: Tue, 5 Jan 2016 23:28:48 +0100 Subject: [PATCH] Move some static methods out of their classes where it makes sense --- openlp/core/lib/renderer.py | 94 +++++++++---------- openlp/core/ui/mainwindow.py | 3 +- openlp/core/ui/servicemanager.py | 58 +++++------- openlp/plugins/songs/forms/songexportform.py | 57 ++++++----- .../songs/lib/importers/foilpresenter.py | 53 ++++++----- .../songs/test_foilpresenterimport.py | 2 +- 6 files changed, 126 insertions(+), 141 deletions(-) diff --git a/openlp/core/lib/renderer.py b/openlp/core/lib/renderer.py index a0dc14117..395c3d469 100644 --- a/openlp/core/lib/renderer.py +++ b/openlp/core/lib/renderer.py @@ -451,43 +451,6 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties): formatted.append(previous_raw) return formatted - @staticmethod - def _get_start_tags(raw_text): - """ - Tests the given text for not closed formatting tags and returns a tuple consisting of three unicode strings:: - - ('{st}{r}Text text text{/r}{/st}', '{st}{r}', '') - - The first unicode string is the text, with correct closing tags. The second unicode string are OpenLP's opening - formatting tags and the third unicode string the html opening formatting tags. - - :param raw_text: The text to test. The text must **not** contain html tags, only OpenLP formatting tags - are allowed:: - {st}{r}Text text text - """ - raw_tags = [] - html_tags = [] - for tag in FormattingTags.get_html_tags(): - if tag['start tag'] == '{br}': - continue - if raw_text.count(tag['start tag']) != raw_text.count(tag['end tag']): - raw_tags.append((raw_text.find(tag['start tag']), tag['start tag'], tag['end tag'])) - html_tags.append((raw_text.find(tag['start tag']), tag['start html'])) - # Sort the lists, so that the tags which were opened first on the first slide (the text we are checking) will be - # opened first on the next slide as well. - raw_tags.sort(key=lambda tag: tag[0]) - html_tags.sort(key=lambda tag: tag[0]) - # Create a list with closing tags for the raw_text. - end_tags = [] - start_tags = [] - for tag in raw_tags: - start_tags.append(tag[1]) - end_tags.append(tag[2]) - end_tags.reverse() - # Remove the indexes. - 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) @@ -522,7 +485,7 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties): if smallest_index == index or highest_index == index: index = smallest_index text = previous_raw.rstrip('
') + separator.join(raw_list[:index + 1]) - text, raw_tags, html_tags = Renderer._get_start_tags(text) + text, raw_tags, html_tags = _get_start_tags(text) formatted.append(text) previous_html = '' previous_raw = '' @@ -557,13 +520,50 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties): 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(' ') +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:: + + ('{st}{r}Text text text{/r}{/st}', '{st}{r}', '') + + The first unicode string is the text, with correct closing tags. The second unicode string are OpenLP's opening + formatting tags and the third unicode string the html opening formatting tags. + + :param raw_text: The text to test. The text must **not** contain html tags, only OpenLP formatting tags + are allowed:: + {st}{r}Text text text + """ + raw_tags = [] + html_tags = [] + for tag in FormattingTags.get_html_tags(): + if tag['start tag'] == '{br}': + continue + if raw_text.count(tag['start tag']) != raw_text.count(tag['end tag']): + raw_tags.append((raw_text.find(tag['start tag']), tag['start tag'], tag['end tag'])) + html_tags.append((raw_text.find(tag['start tag']), tag['start html'])) + # Sort the lists, so that the tags which were opened first on the first slide (the text we are checking) will be + # opened first on the next slide as well. + raw_tags.sort(key=lambda tag: tag[0]) + html_tags.sort(key=lambda tag: tag[0]) + # Create a list with closing tags for the raw_text. + end_tags = [] + start_tags = [] + for tag in raw_tags: + start_tags.append(tag[1]) + end_tags.append(tag[2]) + end_tags.reverse() + # Remove the indexes. + html_tags = [tag[1] for tag in html_tags] + return raw_text + ''.join(end_tags), ''.join(start_tags), ''.join(html_tags) + diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index aed44468e..4427de655 100644 --- a/openlp/core/ui/mainwindow.py +++ b/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) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 06005bb60..3904d28da 100644 --- a/openlp/core/ui/servicemanager.py +++ b/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__.dragMoveEvent = Ui_ServiceManager.drag_enter_event + self.service_manager_list.__class__.dragEnterEvent = lambda x, event: event.accept() + 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,29 +1648,22 @@ 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_parent_item_data(item): - """ - Finds and returns the parent item for any item - - :param item: The service item list item to be checked. - """ - parent_item = item.parent() - if parent_item is None: - return item.data(0, QtCore.Qt.UserRole) - 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 + + +def _get_parent_item_data(item): + """ + Finds and returns the parent item for any item + + :param item: The service item list item to be checked. + """ + parent_item = item.parent() + if parent_item is None: + return item.data(0, QtCore.Qt.UserRole) + else: + return parent_item.data(0, QtCore.Qt.UserRole) + diff --git a/openlp/plugins/songs/forms/songexportform.py b/openlp/plugins/songs/forms/songexportform.py index b9ff2d2f6..31e490aa0 100644 --- a/openlp/plugins/songs/forms/songexportform.py +++ b/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) + diff --git a/openlp/plugins/songs/lib/importers/foilpresenter.py b/openlp/plugins/songs/lib/importers/foilpresenter.py index 2d9c6cd4c..5827086c0 100644 --- a/openlp/plugins/songs/lib/importers/foilpresenter.py +++ b/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 '' - verse_name = FoilPresenter._child(strophe.key) + text = _child(strophe.text_) if hasattr(strophe, 'text_') else '' + 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): - song.song_number = FoilPresenter._child(bucheintrag.nummer) + if _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 '' + diff --git a/tests/functional/openlp_plugins/songs/test_foilpresenterimport.py b/tests/functional/openlp_plugins/songs/test_foilpresenterimport.py index 87e5fc484..2eb55afc3 100644 --- a/tests/functional/openlp_plugins/songs/test_foilpresenterimport.py +++ b/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 = \