← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~suutari-olli/openlp/force-split into lp:openlp

 

Azaziah has proposed merging lp:~suutari-olli/openlp/force-split into lp:openlp.

Requested reviews:
  Tim Bentley (trb143)
Related bugs:
  Bug #858344 in OpenLP: "Split Verse should have option to force the split"
  https://bugs.launchpad.net/openlp/+bug/858344
  Bug #1500368 in OpenLP: "Easy way to Force Split verses"
  https://bugs.launchpad.net/openlp/+bug/1500368
  Bug #1576148 in OpenLP: "Optional Split In Song Editor Does Nothing On Output Display Or Preview"
  https://bugs.launchpad.net/openlp/+bug/1576148

For more details, see:
https://code.launchpad.net/~suutari-olli/openlp/force-split/+merge/296002

- Added "Split" button for Song editor:
  This adds the currently selected Verses tag to
  the given position, thus creating a "Force split"
- Increased Optional Split limit from < 5 to < 51
- Increased test coverage

[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-01-Pull/1579/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-02-Functional-Tests/1490/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-03-Interface-Tests/1428/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tets/1207/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tess/797/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-05a-Code_Analysis/865/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-05b-Test_Coverage/733/
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/lib/renderer.py'
--- openlp/core/lib/renderer.py	2016-05-15 17:33:42 +0000
+++ openlp/core/lib/renderer.py	2016-05-29 14:06:59 +0000
@@ -241,7 +241,7 @@
         elif item.is_capable(ItemCapabilities.CanSoftBreak):
             pages = []
             if '[---]' in text:
-                # Remove two or more option slide breaks next to each other (causing infinite loop).
+                # Remove two or more optional splits next to each other (causing infinite loop).
                 while '\n[---]\n[---]\n' in text:
                     text = text.replace('\n[---]\n[---]\n', '\n[---]\n')
                 while ' [---]' in text:
@@ -249,8 +249,8 @@
                 while '[---] ' in text:
                     text = text.replace('[---] ', '[---]')
                 count = 0
-                # only loop 5 times as there will never be more than 5 incorrect logical splits on a single slide.
-                while True and count < 5:
+                # only loop 50 times as there will never be more than 50 optional splits on a single slide.
+                while True and count < 51:
                     slides = text.split('\n[---]\n', 2)
                     # If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last
                     # for now).

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2016-04-25 11:01:32 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-05-29 14:06:59 +0000
@@ -776,6 +776,55 @@
                         item = QtWidgets.QTableWidgetItem(entry, 0)
                         item.setData(QtCore.Qt.UserRole, temp_ids[row])
                         self.verse_list_widget.setItem(row, 0, item)
+        # This code converts force splits aka. Verse tags inserted during edit to new verses from text.
+        # It is largely copied from def on_verse_edit_all_button_clicked
+        verse_list = ''
+        if self.verse_list_widget.rowCount() > 0:
+            for row in range(self.verse_list_widget.rowCount()):
+                item = self.verse_list_widget.item(row, 0)
+                field = item.data(QtCore.Qt.UserRole)
+                verse_tag = VerseType.translated_name(field[0])
+                verse_num = field[1:]
+                verse_list += '---[%s:%s]---\n' % (verse_tag, verse_num)
+                verse_list += item.text()
+                verse_list += '\n'
+            self.verse_form.set_verse(verse_list)
+        verse_list = self.verse_form.get_all_verses()
+        verse_list = str(verse_list.replace('\r\n', '\n'))
+        self.verse_list_widget.clear()
+        self.verse_list_widget.setRowCount(0)
+        for row in self.find_verse_split.split(verse_list):
+            for match in row.split('---['):
+                for count, parts in enumerate(match.split(']---\n')):
+                    if count == 0:
+                        if len(parts) == 0:
+                            continue
+                        # handling carefully user inputted versetags
+                        separator = parts.find(':')
+                        if separator >= 0:
+                            verse_name = parts[0:separator].strip()
+                            verse_num = parts[separator + 1:].strip()
+                        else:
+                            verse_name = parts
+                            verse_num = '1'
+                        verse_index = VerseType.from_loose_input(verse_name)
+                        verse_tag = VerseType.tags[verse_index]
+                        # Later we need to handle v1a as well.
+                        regex = re.compile(r'\D*(\d+)\D*')
+                        match = regex.match(verse_num)
+                        if match:
+                            verse_num = match.group(1)
+                        else:
+                            verse_num = '1'
+                        verse_def = '%s%s' % (verse_tag, verse_num)
+                    else:
+                        if parts.endswith('\n'):
+                            parts = parts.rstrip('\n')
+                        item = QtWidgets.QTableWidgetItem(parts)
+                        item.setData(QtCore.Qt.UserRole, verse_def)
+                        self.verse_list_widget.setRowCount(self.verse_list_widget.rowCount() + 1)
+                        self.verse_list_widget.setItem(self.verse_list_widget.rowCount() - 1, 0, item)
+        # Finalize edit
         self.tag_rows()
         # Check if all verse tags are used.
         self.on_verse_order_text_changed(self.verse_order_edit.text())

=== modified file 'openlp/plugins/songs/forms/editversedialog.py'
--- openlp/plugins/songs/forms/editversedialog.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/forms/editversedialog.py	2016-05-29 14:06:59 +0000
@@ -44,6 +44,10 @@
         self.split_button.setIcon(build_icon(':/general/general_add.png'))
         self.split_button.setObjectName('split_button')
         self.verse_type_layout.addWidget(self.split_button)
+        self.force_split_button = QtWidgets.QPushButton(edit_verse_dialog)
+        self.force_split_button.setIcon(build_icon(':/general/general_add.png'))
+        self.force_split_button.setObjectName('force_split_button')
+        self.verse_type_layout.addWidget(self.force_split_button)
         self.verse_type_label = QtWidgets.QLabel(edit_verse_dialog)
         self.verse_type_label.setObjectName('verse_type_label')
         self.verse_type_layout.addWidget(self.verse_type_label)
@@ -78,6 +82,8 @@
         self.verse_type_combo_box.setItemText(VerseType.Other, VerseType.translated_names[VerseType.Other])
         self.split_button.setText(UiStrings().Split)
         self.split_button.setToolTip(UiStrings().SplitToolTip)
+        self.force_split_button.setText(translate('SongsPlugin.EditVerseForm', '&Split'))
+        self.force_split_button.setToolTip(translate('SongsPlugin.EditVerseForm', 'Split the verse.'))
         self.insert_button.setText(translate('SongsPlugin.EditVerseForm', '&Insert'))
         self.insert_button.setToolTip(translate('SongsPlugin.EditVerseForm',
                                       'Split a slide into two by inserting a verse splitter.'))

=== modified file 'openlp/plugins/songs/forms/editverseform.py'
--- openlp/plugins/songs/forms/editverseform.py	2016-01-09 16:26:14 +0000
+++ openlp/plugins/songs/forms/editverseform.py	2016-05-29 14:06:59 +0000
@@ -46,6 +46,7 @@
         self.has_single_verse = False
         self.insert_button.clicked.connect(self.on_insert_button_clicked)
         self.split_button.clicked.connect(self.on_split_button_clicked)
+        self.force_split_button.clicked.connect(self.on_force_split_button_clicked)
         self.verse_text_edit.cursorPositionChanged.connect(self.on_cursor_position_changed)
         self.verse_type_combo_box.currentIndexChanged.connect(self.on_verse_type_combo_box_changed)
 
@@ -64,7 +65,7 @@
 
     def on_split_button_clicked(self):
         """
-        The split button has been pressed
+        The (optional) split button has been pressed
         """
         text = self.verse_text_edit.toPlainText()
         position = self.verse_text_edit.textCursor().position()
@@ -76,6 +77,39 @@
         self.verse_text_edit.insertPlainText(insert_string)
         self.verse_text_edit.setFocus()
 
+    def on_force_split_button_clicked(self, verse_tag, verse_num=1):
+        """
+        The force split button has been pressed
+        Find the last used verse tag by using given position and regex.
+        Then insert the tag and create a new line if required.
+        """
+        position = self.verse_text_edit.textCursor().position()
+        text = self.verse_text_edit.toPlainText()
+        position = text.rfind('---[', 0, position)
+        # If  editing single verse or ---[ not found, get value of the current verse and insert it.
+        # This will insert the verse tag if editing a single verse or all  verses and no verse tag is present.
+        if self.has_single_verse or position == -1:
+            verse_type_index = self.verse_type_combo_box.currentIndex()
+            self.insert_verse(VerseType.tags[verse_type_index], self.verse_number_box.value())
+        # Find & Duplicate the currently selected verses tag and insert it.
+        else:
+            text = text[position:]
+            position = text.find(']---')
+            if position == -1:
+                return
+            text = text[:position + 4]
+            match = re.match(('---\[.*\]---'), text)
+            insert_string = match.group()
+            # Reset text & position data for checking the proper inserting place & is newline required.
+            text = self.verse_text_edit.toPlainText()
+            position = self.verse_text_edit.textCursor().position()
+            if position and text[position - 1] != '\n':
+                insert_string = '\n' + insert_string
+            if position == len(text) or text[position] != '\n':
+                insert_string += '\n'
+            self.verse_text_edit.insertPlainText(insert_string)
+            self.verse_text_edit.setFocus()
+
     def on_insert_button_clicked(self):
         """
         The insert button has been pressed
@@ -99,13 +133,11 @@
         """
         Adjusts the verse number SpinBox in regard to the selected verse type and the cursor's position.
         """
-        if self.has_single_verse:
-            return
         position = self.verse_text_edit.textCursor().position()
         text = self.verse_text_edit.toPlainText()
         verse_name = VerseType.translated_names[
             self.verse_type_combo_box.currentIndex()]
-        if not text:
+        if not text or self.has_single_verse:
             return
         position = text.rfind('---[%s' % verse_name, 0, position)
         if position == -1:
@@ -115,7 +147,12 @@
         position = text.find(']---')
         if position == -1:
             return
-        text = text[:position + 4]
+        # If editing a Single verse, set the text to match current verse, otherwise increase the suggested verse.
+        # This seems to only take effect when adding a split in middle of the sentence.
+        if self.has_single_verse:
+            text = text[:position]
+        else:
+            text = text[:position + 4]
         match = VERSE_REGEX.match(text)
         if match:
             try:
@@ -153,6 +190,7 @@
     def get_verse(self):
         """
         Extract the verse text
+        If you are looking for the save method, check out songs\forms\editsongform.py def on_verse_edit_button_clicked
 
         :return: The text
         """
@@ -162,6 +200,7 @@
     def get_all_verses(self):
         """
         Extract all the verses
+        If you are looking for the save method, check out forms\editsongform.py def on_verse_edit_all_button_clicked
 
         :return: The text
         """

=== modified file 'tests/functional/openlp_plugins/songs/test_editverseform.py'
--- tests/functional/openlp_plugins/songs/test_editverseform.py	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_editverseform.py	2016-05-29 14:06:59 +0000
@@ -55,16 +55,29 @@
 
     def update_suggested_verse_number_test(self):
         """
-        Test that update_suggested_verse_number() has no effect when editing a single verse
+        Test that update_suggested_verse_number() does not change the verse number for force split in edit single verse.
         """
         # GIVEN some input values
         self.edit_verse_form.has_single_verse = True
         self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=0)
         self.edit_verse_form.verse_text_edit.toPlainText = MagicMock(return_value='Text')
-        self.edit_verse_form.verse_number_box.setValue(3)
+        self.edit_verse_form.verse_number_box.setValue(1)
 
         # WHEN the method is called
         self.edit_verse_form.update_suggested_verse_number()
 
         # THEN the verse number must not be changed
-        self.assertEqual(3, self.edit_verse_form.verse_number_box.value(), 'The verse number should be 3')
+        self.assertEqual(1, self.edit_verse_form.verse_number_box.value(), 'The verse number should be 1')
+
+    def on_insert_button_clicked_test(self):
+        """
+        Test that insert_verse is called on def on_insert_button_clicked
+        """
+        # GIVEN required functions
+        self.edit_verse_form.insert_verse = MagicMock()
+
+        # WHEN the method is called
+        self.edit_verse_form.on_insert_button_clicked()
+
+        # THEN the verse number must not be changed
+        self.assertEqual(1, self.edit_verse_form.insert_verse.call_count, 'The insert_verse should had been called')


Follow ups