Trunk

openlp/core/ui/servicemanager.py

@@ -235,7 +235,8 @@ class Ui_ServiceManager(object):
         self.edit_action = create_widget_action(self.menu, text=translate('OpenLP.ServiceManager', '&Edit Item'),
                                                 icon=':/general/general_edit.png', triggers=self.remote_edit)
         self.rename_action = create_widget_action(self.menu, text=translate('OpenLP.ServiceManager', '&Rename...'),
-                                                  icon=':/general/general_edit.png', triggers=self.on_service_item_rename)
+                                                  icon=':/general/general_edit.png',
+                                                  triggers=self.on_service_item_rename)
         self.maintain_action = create_widget_action(self.menu, text=translate('OpenLP.ServiceManager', '&Reorder Item'),
                                                     icon=':/general/general_edit.png',
                                                     triggers=self.on_service_item_edit_form)
@@ -1488,9 +1489,11 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ServiceManage
             if new_item:
                 self.add_service_item(new_item, replace=True)
 
-    def on_service_item_rename(self):
+    def on_service_item_rename(self, field=None):
         """
         Opens a dialog to rename the service item.
+
+        :param field: Not used, but PyQt needs this.
         """
         item = self.find_service_item()[0]
         if not self.service_items[item]['service_item'].is_capable(ItemCapabilities.CanEditTitle):

openlp/plugins/songs/forms/duplicatesongremovalform.py

@@ -31,6 +31,7 @@ The duplicate song removal logic for OpenLP.
 """
 
 import logging
+import multiprocessing
 import os
 
 from PyQt4 import QtCore, QtGui
@@ -45,6 +46,18 @@ from openlp.plugins.songs.lib.songcompare import songs_probably_equal
 log = logging.getLogger(__name__)
 
 
+def song_generator(songs):
+    """
+    This is a generator function to return tuples of two songs. When completed then all songs have once been returned
+    combined with any other songs.
+
+    :param songs: All songs in the database.
+    """
+    for outer_song_counter in range(len(songs) - 1):
+        for inner_song_counter in range(outer_song_counter + 1, len(songs)):
+            yield (songs[outer_song_counter], songs[inner_song_counter])
+
+
 class DuplicateSongRemovalForm(OpenLPWizard, RegistryProperties):
     """
     This is the Duplicate Song Removal Wizard. It provides functionality to search for and remove duplicate songs
@@ -167,24 +180,31 @@ class DuplicateSongRemovalForm(OpenLPWizard, RegistryProperties):
                 max_progress_count = max_songs * (max_songs - 1) // 2
                 self.duplicate_search_progress_bar.setMaximum(max_progress_count)
                 songs = self.plugin.manager.get_all_objects(Song)
-                for outer_song_counter in range(max_songs - 1):
+                # Create a worker/process pool to check the songs.
-                    for inner_song_counter in range(outer_song_counter + 1, max_songs):
+                process_number = max(1, multiprocessing.cpu_count() - 1)
-                        if songs_probably_equal(songs[outer_song_counter], songs[inner_song_counter]):
+                pool = multiprocessing.Pool(process_number)
-                            duplicate_added = self.add_duplicates_to_song_list(
+                result = pool.imap_unordered(songs_probably_equal, song_generator(songs), 30)
-                                songs[outer_song_counter], songs[inner_song_counter])
+                # Do not accept any further tasks. Also this closes the processes if all tasks are done.
-                            if duplicate_added:
+                pool.close()
-                                self.found_duplicates_edit.appendPlainText(
+                # While the processes are still working, start to look at the results.
-                                    songs[outer_song_counter].title + "  =  " + songs[inner_song_counter].title)
+                for song_tuple in result:
-                        self.duplicate_search_progress_bar.setValue(self.duplicate_search_progress_bar.value() + 1)
+                    self.duplicate_search_progress_bar.setValue(self.duplicate_search_progress_bar.value() + 1)
-                        # The call to process_events() will keep the GUI responsive.
+                    # The call to process_events() will keep the GUI responsive.
-                        self.application.process_events()
+                    self.application.process_events()
-                        if self.break_search:
+                    if self.break_search:
-                            return
+                        pool.terminate()
+                        return
+                    if song_tuple is None:
+                        continue
+                    song1, song2 = song_tuple
+                    duplicate_added = self.add_duplicates_to_song_list(song1, song2)
+                    if duplicate_added:
+                        self.found_duplicates_edit.appendPlainText(song1.title + "  =  " + song2.title)
                 self.review_total_count = len(self.duplicate_song_list)
-                if self.review_total_count == 0:
+                if self.duplicate_song_list:
-                    self.notify_no_duplicates()
-                else:
                     self.button(QtGui.QWizard.NextButton).show()
+                else:
+                    self.notify_no_duplicates()
             finally:
                 self.application.set_normal_cursor()
         elif page_id == self.review_page_id:

openlp/plugins/songs/lib/songcompare.py

@@ -52,13 +52,13 @@ MIN_BLOCK_SIZE = 70
 MAX_TYPO_SIZE = 3
 
 
-def songs_probably_equal(song1, song2):
+def songs_probably_equal(song_tupel):
     """
     Calculate and return whether two songs are probably equal.
 
-    :param song1: The first song to compare.
+    :param song_tupel: A tuple of two songs to compare.
-    :param song2:  The second song to compare.
     """
+    song1, song2 = song_tupel
     if len(song1.search_lyrics) < len(song2.search_lyrics):
         small = song1.search_lyrics
         large = song2.search_lyrics
@@ -75,18 +75,19 @@ def songs_probably_equal(song1, song2):
     for element in diff_no_typos:
         if element[0] == "equal" and _op_length(element) >= MIN_BLOCK_SIZE:
             length_of_equal_blocks += _op_length(element)
+
     if length_of_equal_blocks >= MIN_BLOCK_SIZE:
-        return True
+        return song1, song2
     # Check 2: Similarity based on the relative length of the longest equal block.
     # Calculate the length of the largest equal block of the diff set.
     length_of_longest_equal_block = 0
     for element in diff_no_typos:
         if element[0] == "equal" and _op_length(element) > length_of_longest_equal_block:
             length_of_longest_equal_block = _op_length(element)
-    if length_of_equal_blocks >= MIN_BLOCK_SIZE or length_of_longest_equal_block > len(small) * 2 // 3:
+    if length_of_longest_equal_block > len(small) * 2 // 3:
-        return True
+        return song1, song2
     # Both checks failed. We assume the songs are not equal.
-    return False
+    return None
 
 
 def _op_length(opcode):

openlp/plugins/songs/lib/xml.py

@@ -675,6 +675,7 @@ class OpenLyrics(object):
         sxml = SongXML()
         verses = {}
         verse_def_list = []
+        verse_order = self._text(properties.verseOrder).split(' ') if hasattr(properties, 'verseOrder') else []
         try:
             lyrics = song_xml.lyrics
         except AttributeError:
@@ -717,13 +718,17 @@ class OpenLyrics(object):
             else:
                 verses[(verse_tag, verse_number, lang, translit, verse_part)] = text
                 verse_def_list.append((verse_tag, verse_number, lang, translit, verse_part))
+            # Update verse order when the verse name has changed
+            if verse_def != verse_tag + verse_number + verse_part:
+                for i in range(len(verse_order)):
+                    if verse_order[i] == verse_def:
+                        verse_order[i] = verse_tag + verse_number + verse_part
         # We have to use a list to keep the order, as dicts are not sorted.
         for verse in verse_def_list:
             sxml.add_verse_to_lyrics(verse[0], verse[1], verses[verse], verse[2])
         song_obj.lyrics = str(sxml.extract_xml(), 'utf-8')
         # Process verse order
-        if hasattr(properties, 'verseOrder'):
+        song_obj.verse_order = ' '.join(verse_order)
-            song_obj.verse_order = self._text(properties.verseOrder)
 
     def _process_songbooks(self, properties, song):
         """

tests/functional/openlp_core_lib/test_renderer.py

@@ -49,7 +49,7 @@ class TestRenderer(TestCase):
 
     def setUp(self):
         """
-        Set up the components need for all tests.
+        Set up the components need for all tests
         """
         # Mocked out desktop object
         self.desktop = MagicMock()
@@ -67,7 +67,7 @@ class TestRenderer(TestCase):
 
     def initial_renderer_test(self):
         """
-        Test the initial renderer state .
+        Test the initial renderer state
         """
         # GIVEN: A new renderer instance.
         renderer = Renderer()
@@ -77,7 +77,7 @@ class TestRenderer(TestCase):
 
     def default_screen_layout_test(self):
         """
-        Test the default layout calculations.
+        Test the default layout calculations
         """
         # GIVEN: A new renderer instance.
         renderer = Renderer()
@@ -87,3 +87,35 @@ class TestRenderer(TestCase):
         self.assertEqual(renderer.height, 768, 'The base renderer should be a live controller')
         self.assertEqual(renderer.screen_ratio, 0.75, 'The base renderer should be a live controller')
         self.assertEqual(renderer.footer_start, 691, 'The base renderer should be a live controller')
+
+    def _get_start_tags_test(self):
+        """
+        Test the _get_start_tags() method
+        """
+        # GIVEN: A new renderer instance. Broken raw_text (missing closing tags).
+        renderer = Renderer()
+        given_raw_text = '{st}{r}Text text text'
+        expected_tuple = ('{st}{r}Text text text{/r}{/st}', '{st}{r}',
+                          '<strong><span style="-webkit-text-fill-color:red">')
+
+        # WHEN:
+        result = renderer._get_start_tags(given_raw_text)
+
+        # THEN: Check if the correct tuple is returned.
+        self.assertEqual(result, expected_tuple), 'A tuple should be returned containing the text with correct ' \
+            'tags, the opening tags, and the opening html tags.'
+
+    def _word_split_test(self):
+        """
+        Test the _word_split() method
+        """
+        # GIVEN: A line of text
+        renderer = Renderer()
+        given_line = 'beginning asdf \n end asdf'
+        expected_words = ['beginning', 'asdf', 'end', 'asdf']
+
+        # WHEN: Split the line
+        result_words = renderer._words_split(given_line)
+
+        # THEN: The word lists should be the same.
+        self.assertListEqual(result_words, expected_words)

tests/functional/openlp_core_lib/test_ui.py

@@ -131,3 +131,17 @@ class TestUi(TestCase):
         for text in [UiStrings().Top, UiStrings().Middle, UiStrings().Bottom]:
             self.assertTrue(combo.findText(text) >= 0)
 
+    def test_create_horizontal_adjusting_combo_box(self):
+        """
+        Test creating a horizontal adjusting combo box
+        """
+        # GIVEN: A dialog
+        dialog = QtGui.QDialog()
+
+        # WHEN: We create the combobox
+        combo = create_horizontal_adjusting_combo_box(dialog, 'combo1')
+
+        # THEN: We should get a ComboBox
+        self.assertIsInstance(combo, QtGui.QComboBox)
+        self.assertEqual('combo1', combo.objectName())
+        self.assertEqual(QtGui.QComboBox.AdjustToMinimumContentsLength, combo.sizeAdjustPolicy())

tests/functional/openlp_plugins/songs/test_lib.py

@@ -96,10 +96,10 @@ class TestLib(TestCase):
         self.song2.search_lyrics = self.full_lyrics
 
         # WHEN: We compare those songs for equality.
-        result = songs_probably_equal(self.song1, self.song2)
+        result = songs_probably_equal((self.song1, self.song2))
 
-        # THEN: The result should be True.
+        # THEN: The result should be a tuple..
-        assert result is True, 'The result should be True'
+        assert result == (self.song1, self.song2), 'The result should be the tuble of songs'
 
     def songs_probably_equal_short_song_test(self):
         """
@@ -110,10 +110,10 @@ class TestLib(TestCase):
         self.song2.search_lyrics = self.short_lyrics
 
         # WHEN: We compare those songs for equality.
-        result = songs_probably_equal(self.song1, self.song2)
+        result = songs_probably_equal((self.song1, self.song2))
 
-        # THEN: The result should be True.
+        # THEN: The result should be a tuple..
-        assert result is True, 'The result should be True'
+        assert result == (self.song1, self.song2), 'The result should be the tuble of songs'
 
     def songs_probably_equal_error_song_test(self):
         """
@@ -124,10 +124,11 @@ class TestLib(TestCase):
         self.song2.search_lyrics = self.error_lyrics
 
         # WHEN: We compare those songs for equality.
-        result = songs_probably_equal(self.song1, self.song2)
+        result = songs_probably_equal((self.song1, self.song2))
+
+        # THEN: The result should be a tuple of songs..
+        assert result == (self.song1, self.song2), 'The result should be the tuble of songs'
 
-        # THEN: The result should be True.
-        assert result is True, 'The result should be True'
 
     def songs_probably_equal_different_song_test(self):
         """
@@ -138,10 +139,10 @@ class TestLib(TestCase):
         self.song2.search_lyrics = self.different_lyrics
 
         # WHEN: We compare those songs for equality.
-        result = songs_probably_equal(self.song1, self.song2)
+        result = songs_probably_equal((self.song1, self.song2))
 
-        # THEN: The result should be False.
+        # THEN: The result should be None.
-        assert result is False, 'The result should be False'
+        assert result is None, 'The result should be None'
 
     def remove_typos_beginning_test(self):
         """