Merge lp:~trb143/openlp/splitter into lp:openlp

[Tim Bentley]

Add option to add a split to a song
This will split the verse when added to the service but keep the verse together for editing.
Useful for the 10 line hymn verses which need 2 slides to display.

Fix some iffy spelling

lp:~trb143/openlp/splitter (revision 2738)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2182/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2085/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1972/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1342/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1178/
[SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/308/
[FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/150/

[Raoul Snyman (raoul-snyman)]

As per IRC, mrs_fly's suggestion was "Auto Split" and "Force Split". I'm going to e-mail the mailing list and see what others think.

[Tomas Groth (tomasgroth)]

The openlyrics export does handle the new spilt, which I believe it should. The import should of course also be able to restore it. I'll admit I'm not sure how it should be done...

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

Let's call the buttons "Overflow Split" and "Forced Split"

[Tomas Groth (tomasgroth)]

The openlyrics import/export still needs to handle the new spilt...

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

I would prefer a different splitter, but that is just personal preference. Other than that, this looks fine to me.

Also, I'm not entirely sure that the <p> tag is a valid element in OpenLyrics, but let's roll with it and we can always sort out OpenLyrics later.

review: Approve

=== modified file 'openlp/core/common/uistrings.py'
--- openlp/core/common/uistrings.py	2017-09-06 21:36:31 +0000
+++ openlp/core/common/uistrings.py	2017-10-01 20:11:51 +0000
@@ -147,7 +147,7 @@
         self.SaveService = translate('OpenLP.Ui', 'Save Service')
         self.Service = translate('OpenLP.Ui', 'Service')
         self.ShortResults = translate('OpenLP.Ui', 'Please type more text to use \'Search As You Type\'')
-        self.Split = translate('OpenLP.Ui', 'Optional &Split')
+        self.Split = translate('OpenLP.Ui', 'Overflow &Split')
         self.SplitToolTip = translate('OpenLP.Ui',
                                       'Split a slide into two only if it does not fit on the screen as one slide.')
         self.StartingImport = translate('OpenLP.Ui', 'Starting import...')
 
=== modified file 'openlp/core/lib/renderer.py'
--- openlp/core/lib/renderer.py	2017-09-26 16:39:13 +0000
+++ openlp/core/lib/renderer.py	2017-10-01 20:11:51 +0000
@@ -243,6 +243,9 @@
         elif item.is_capable(ItemCapabilities.CanSoftBreak):
             pages = []
             if '[---]' in text:
+                # Remove Overflow split if at start of the text
+                if text.startswith('[---]'):
+                    text = text[5:]
                 # Remove two or more option slide breaks next to each other (causing infinite loop).
                 while '\n[---]\n[---]\n' in text:
                     text = text.replace('\n[---]\n[---]\n', '\n[---]\n')
 
=== modified file 'openlp/core/ui/lib/listpreviewwidget.py'
--- openlp/core/ui/lib/listpreviewwidget.py	2016-12-31 11:01:36 +0000
+++ openlp/core/ui/lib/listpreviewwidget.py	2017-10-01 20:11:51 +0000
@@ -209,21 +209,21 @@
         Switches to the given row.
         """
         # Retrieve setting
-        autoscrolling = Settings().value('advanced/autoscrolling')
+        auto_scrolling = Settings().value('advanced/autoscrolling')
         # Check if auto-scroll disabled (None) and validate value as dict containing 'dist' and 'pos'
         # 'dist' represents the slide to scroll to relative to the new slide (-1 = previous, 0 = current, 1 = next)
         # 'pos' represents the vert position of of the slide (0 = in view, 1 = top, 2 = middle, 3 = bottom)
-        if not (isinstance(autoscrolling, dict) and 'dist' in autoscrolling and 'pos' in autoscrolling and
-                isinstance(autoscrolling['dist'], int) and isinstance(autoscrolling['pos'], int)):
+        if not (isinstance(auto_scrolling, dict) and 'dist' in auto_scrolling and 'pos' in auto_scrolling and
+                isinstance(auto_scrolling['dist'], int) and isinstance(auto_scrolling['pos'], int)):
             return
         # prevent scrolling past list bounds
-        scroll_to_slide = slide + autoscrolling['dist']
+        scroll_to_slide = slide + auto_scrolling['dist']
         if scroll_to_slide < 0:
             scroll_to_slide = 0
         if scroll_to_slide >= self.slide_count():
             scroll_to_slide = self.slide_count() - 1
         # Scroll to item if possible.
-        self.scrollToItem(self.item(scroll_to_slide, 0), autoscrolling['pos'])
+        self.scrollToItem(self.item(scroll_to_slide, 0), auto_scrolling['pos'])
         self.selectRow(slide)
 
     def current_slide_number(self):
 
=== modified file 'openlp/plugins/songs/forms/duplicatesongremovalform.py'
--- openlp/plugins/songs/forms/duplicatesongremovalform.py	2017-06-09 06:06:49 +0000
+++ openlp/plugins/songs/forms/duplicatesongremovalform.py	2017-10-01 20:11:51 +0000
@@ -25,14 +25,13 @@
 
 import logging
 import multiprocessing
-import os
 
 from PyQt5 import QtCore, QtWidgets
 
 from openlp.core.common import Registry, RegistryProperties, translate
 from openlp.core.ui.lib.wizard import OpenLPWizard, WizardStrings
 from openlp.plugins.songs.lib import delete_song
-from openlp.plugins.songs.lib.db import Song, MediaFile
+from openlp.plugins.songs.lib.db import Song
 from openlp.plugins.songs.forms.songreviewwidget import SongReviewWidget
 from openlp.plugins.songs.lib.songcompare import songs_probably_equal
 
 
=== modified file 'openlp/plugins/songs/forms/editversedialog.py'
--- openlp/plugins/songs/forms/editversedialog.py	2017-07-04 23:13:51 +0000
+++ openlp/plugins/songs/forms/editversedialog.py	2017-10-01 20:11:51 +0000
@@ -42,10 +42,14 @@
         self.dialog_layout.addWidget(self.verse_text_edit)
         self.verse_type_layout = QtWidgets.QHBoxLayout()
         self.verse_type_layout.setObjectName('verse_type_layout')
-        self.split_button = QtWidgets.QPushButton(edit_verse_dialog)
-        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.forced_split_button = QtWidgets.QPushButton(edit_verse_dialog)
+        self.forced_split_button.setIcon(build_icon(':/general/general_add.png'))
+        self.forced_split_button.setObjectName('forced_split_button')
+        self.verse_type_layout.addWidget(self.forced_split_button)
+        self.overflow_split_button = QtWidgets.QPushButton(edit_verse_dialog)
+        self.overflow_split_button.setIcon(build_icon(':/general/general_add.png'))
+        self.overflow_split_button.setObjectName('overflow_split_button')
+        self.verse_type_layout.addWidget(self.overflow_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)
@@ -93,8 +97,11 @@
         self.verse_type_combo_box.setItemText(VerseType.Intro, VerseType.translated_names[VerseType.Intro])
         self.verse_type_combo_box.setItemText(VerseType.Ending, VerseType.translated_names[VerseType.Ending])
         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.overflow_split_button.setText(UiStrings().Split)
+        self.overflow_split_button.setToolTip(UiStrings().SplitToolTip)
+        self.forced_split_button.setText(translate('SongsPlugin.EditVerseForm', '&Forced Split'))
+        self.forced_split_button.setToolTip(translate('SongsPlugin.EditVerseForm', 'Split the verse when displayed '
+                                                                                   'regardless of the screen size.'))
         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	2017-06-04 12:14:23 +0000
+++ openlp/plugins/songs/forms/editverseform.py	2017-10-01 20:11:51 +0000
@@ -48,12 +48,13 @@
         self.setupUi(self)
         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.overflow_split_button.clicked.connect(self.on_overflow_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)
+        self.forced_split_button.clicked.connect(self.on_forced_split_button_clicked)
         if Settings().value('songs/enable chords'):
-            self.transpose_down_button.clicked.connect(self.on_transepose_down_button_clicked)
-            self.transpose_up_button.clicked.connect(self.on_transepose_up_button_clicked)
+            self.transpose_down_button.clicked.connect(self.on_transpose_down_button_clicked)
+            self.transpose_up_button.clicked.connect(self.on_transpose_up_button_clicked)
 
     def insert_verse(self, verse_tag, verse_num=1):
         """
@@ -68,13 +69,27 @@
         self.verse_text_edit.insertPlainText('---[{tag}:{number}]---\n'.format(tag=verse_tag, number=verse_num))
         self.verse_text_edit.setFocus()
 
-    def on_split_button_clicked(self):
-        """
-        The split button has been pressed
+    def on_overflow_split_button_clicked(self):
+        """
+        The optional split button has been pressed so we need add the split
+        """
+        self._add_splitter_to_text('[---]')
+
+    def on_forced_split_button_clicked(self):
+        """
+        The force split button has been pressed so we need add the split
+        """
+        self._add_splitter_to_text('[--}{--]')
+
+    def _add_splitter_to_text(self, insert_string):
+        """
+        Add a custom splitter to the song text
+
+        :param insert_string: The string to insert
+        :return:
         """
         text = self.verse_text_edit.toPlainText()
         position = self.verse_text_edit.textCursor().position()
-        insert_string = '[---]'
         if position and text[position - 1] != '\n':
             insert_string = '\n' + insert_string
         if position == len(text) or text[position] != '\n':
@@ -101,7 +116,7 @@
         """
         self.update_suggested_verse_number()
 
-    def on_transepose_up_button_clicked(self):
+    def on_transpose_up_button_clicked(self):
         """
         The transpose up button clicked
         """
@@ -118,7 +133,7 @@
         self.verse_text_edit.setFocus()
         self.verse_text_edit.moveCursor(QtGui.QTextCursor.End)
 
-    def on_transepose_down_button_clicked(self):
+    def on_transpose_down_button_clicked(self):
         """
         The transpose down button clicked
         """
 
=== modified file 'openlp/plugins/songs/lib/__init__.py'
--- openlp/plugins/songs/lib/__init__.py	2017-09-30 20:16:30 +0000
+++ openlp/plugins/songs/lib/__init__.py	2017-10-01 20:11:51 +0000
@@ -546,12 +546,12 @@
     song_plugin.manager.delete_object(Song, song_id)
 
 
-def transpose_lyrics(lyrics, transepose_value):
+def transpose_lyrics(lyrics, transpose_value):
     """
-    Transepose lyrics
+    Transpose lyrics
 
-    :param lyrcs: The lyrics to be transposed
-    :param transepose_value: The value to transpose the lyrics with
+    :param lyrics: The lyrics to be transposed
+    :param transpose_value: The value to transpose the lyrics with
     :return: The transposed lyrics
     """
     # Split text by verse delimiter - both normal and optional
@@ -562,16 +562,17 @@
         if verse.startswith('---[') or verse == '[---]':
             transposed_lyrics += verse
         else:
-            transposed_lyrics += transpose_verse(verse, transepose_value, notation)
+            transposed_lyrics += transpose_verse(verse, transpose_value, notation)
     return transposed_lyrics
 
 
-def transpose_verse(verse_text, transepose_value, notation):
+def transpose_verse(verse_text, transpose_value, notation):
     """
-    Transepose lyrics
+    Transpose Verse
 
-    :param lyrcs: The lyrics to be transposed
-    :param transepose_value: The value to transpose the lyrics with
+    :param verse_text: The lyrics to be transposed
+    :param transpose_value: The value to transpose the lyrics with
+    :param notation: which notation to use
     :return: The transposed lyrics
     """
     if '[' not in verse_text:
@@ -589,11 +590,11 @@
             if word == ']':
                 in_tag = False
                 transposed_lyrics += word
-            elif word == '/':
+            elif word == '/' or word == '--}{--':
                 transposed_lyrics += word
             else:
                 # This MUST be a chord
-                transposed_lyrics += transpose_chord(word, transepose_value, notation)
+                transposed_lyrics += transpose_chord(word, transpose_value, notation)
     # If still inside a chord tag something is wrong!
     if in_tag:
         return verse_text
@@ -629,36 +630,36 @@
     for i in range(0, len(chord_split)):
         if i > 0:
             transposed_chord += '/'
-        currentchord = chord_split[i]
-        if currentchord and currentchord[0] == '(':
+        current_chord = chord_split[i]
+        if current_chord and current_chord[0] == '(':
             transposed_chord += '('
-            if len(currentchord) > 1:
-                currentchord = currentchord[1:]
+            if len(current_chord) > 1:
+                current_chord = current_chord[1:]
             else:
-                currentchord = ''
-        if len(currentchord) > 0:
-            if len(currentchord) > 1:
-                if '#b'.find(currentchord[1]) == -1:
-                    note = currentchord[0:1]
-                    rest = currentchord[1:]
+                current_chord = ''
+        if len(current_chord) > 0:
+            if len(current_chord) > 1:
+                if '#b'.find(current_chord[1]) == -1:
+                    note = current_chord[0:1]
+                    rest = current_chord[1:]
                 else:
-                    note = currentchord[0:2]
-                    rest = currentchord[2:]
+                    note = current_chord[0:2]
+                    rest = current_chord[2:]
             else:
-                note = currentchord
+                note = current_chord
                 rest = ''
-            notenumber = notes_flat.index(note) if note not in notes_sharp else notes_sharp.index(note)
-            notenumber += transpose_value
-            while notenumber > 11:
-                notenumber -= 12
-            while notenumber < 0:
-                notenumber += 12
+            note_number = notes_flat.index(note) if note not in notes_sharp else notes_sharp.index(note)
+            note_number += transpose_value
+            while note_number > 11:
+                note_number -= 12
+            while note_number < 0:
+                note_number += 12
             if i == 0:
-                current_chord = notes_sharp[notenumber] if notes_preferred[notenumber] == '#' else notes_flat[
-                    notenumber]
+                current_chord = notes_sharp[note_number] if notes_preferred[note_number] == '#' else notes_flat[
+                    note_number]
                 last_chord = current_chord
             else:
-                current_chord = notes_flat[notenumber] if last_chord not in notes_sharp else notes_sharp[notenumber]
+                current_chord = notes_flat[note_number] if last_chord not in notes_sharp else notes_sharp[note_number]
             if not (note not in notes_flat and note not in notes_sharp):
                 transposed_chord += current_chord + rest
             else:
 
=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py	2017-09-30 20:16:30 +0000
+++ openlp/plugins/songs/lib/mediaitem.py	2017-10-01 20:11:51 +0000
@@ -577,7 +577,7 @@
         if not song.verse_order.strip():
             for verse in verse_list:
                 # We cannot use from_loose_input() here, because database is supposed to contain English lowercase
-                # singlechar tags.
+                # single char tags.
                 verse_tag = verse[0]['type']
                 verse_index = None
                 if len(verse_tag) > 1:
@@ -588,7 +588,9 @@
                     verse_index = VerseType.from_tag(verse_tag)
                 verse_tag = VerseType.translated_tags[verse_index].upper()
                 verse_def = '{tag}{label}'.format(tag=verse_tag, label=verse[0]['label'])
-                service_item.add_from_text(str(verse[1]), verse_def)
+                force_verse = verse[1].split('[--}{--]\n')
+                for split_verse in force_verse:
+                    service_item.add_from_text(split_verse, verse_def)
         else:
             # Loop through the verse list and expand the song accordingly.
             for order in song.verse_order.lower().split():
@@ -603,7 +605,9 @@
                             verse_index = VerseType.from_tag(verse[0]['type'])
                         verse_tag = VerseType.translated_tags[verse_index]
                         verse_def = '{tag}{label}'.format(tag=verse_tag, label=verse[0]['label'])
-                        service_item.add_from_text(verse[1], verse_def)
+                        force_verse = verse[1].split('[--}{--]\n')
+                        for split_verse in force_verse:
+                            service_item.add_from_text(split_verse, verse_def)
         service_item.title = song.title
         author_list = self.generate_footer(service_item, song)
         service_item.data_string = {'title': song.search_title, 'authors': ', '.join(author_list)}
 
=== modified file 'openlp/plugins/songs/lib/openlyricsxml.py'
--- openlp/plugins/songs/lib/openlyricsxml.py	2017-09-13 06:08:38 +0000
+++ openlp/plugins/songs/lib/openlyricsxml.py	2017-10-01 20:11:51 +0000
@@ -71,6 +71,7 @@
 
 NAMESPACE = 'http://openlyrics.info/namespace/2009/song'
 NSMAP = '{{' + NAMESPACE + '}}{tag}'
+NEWPAGETAG = '<p style="page-break-after: always;"/>'
 
 
 class SongXML(object):
@@ -281,7 +282,7 @@
         tags_element = None
         match = re.search('\{/?\w+\}', song.lyrics, re.UNICODE)
         if match:
-            # Named 'format_' - 'format' is built-in fuction in Python.
+            # Named 'format_' - 'format' is built-in function in Python.
             format_ = etree.SubElement(song_xml, 'format')
             tags_element = etree.SubElement(format_, 'tags')
             tags_element.set('application', 'OpenLP')
@@ -472,6 +473,7 @@
             text = text.replace('{{/{tag}}}'.format(tag=tag), '</tag>')
         # Replace \n with <br/>.
         text = text.replace('\n', '<br/>')
+        text = text.replace('[--}{--]', NEWPAGETAG)
         element = etree.XML('<lines>{text}</lines>'.format(text=text))
         verse_element.append(element)
         return element
@@ -634,6 +636,9 @@
             if element.tail:
                 text += element.tail
             return text
+        elif newlines and element.tag == NSMAP.format(tag='p') and 'page-break-after' in str(element.attrib):
+            text += '[--}{--]'
+            return text
         # Start formatting tag.
         if element.tag == NSMAP.format(tag='tag'):
             text += '{{{name}}}'.format(name=element.get('name'))
 
=== modified file 'tests/functional/openlp_plugins/songs/test_editverseform.py'
--- tests/functional/openlp_plugins/songs/test_editverseform.py	2017-05-30 20:06:27 +0000
+++ tests/functional/openlp_plugins/songs/test_editverseform.py	2017-10-01 20:11:51 +0000
@@ -72,3 +72,31 @@
 
         # 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')
+
+    def test_on_divide_split_button_clicked(self):
+        """
+        Test that divide adds text at the correct position
+        """
+        # GIVEN some input values
+        self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=4)
+        self.edit_verse_form.verse_text_edit.setPlainText('Text\n')
+
+        # WHEN the method is called
+        self.edit_verse_form.on_forced_split_button_clicked()
+        # THEN the verse number must not be changed
+        self.assertEqual('[--}{--]\nText\n', self.edit_verse_form.verse_text_edit.toPlainText(),
+                         'The verse number should be [--}{--]\nText\n')
+
+    def test_on_split_button_clicked(self):
+        """
+        Test that divide adds text at the correct position
+        """
+        # GIVEN some input values
+        self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=4)
+        self.edit_verse_form.verse_text_edit.setPlainText('Text\n')
+
+        # WHEN the method is called
+        self.edit_verse_form.on_overflow_split_button_clicked()
+        # THEN the verse number must not be changed
+        self.assertEqual('[---]\nText\n', self.edit_verse_form.verse_text_edit.toPlainText(),
+                         'The verse number should be [---]\nText\n')