Properly detect chords, support >5 optional splits

- Properly detect only chords in square brackets
- Properly detect Bibles so that custom slides are rendered as text items
- Make more than 5 optional splits work (fixes #278)

openlp/core/display/render.py

@@ -45,14 +45,15 @@ from openlp.core.lib.formattingtags import FormattingTags
 
 log = logging.getLogger(__name__)
 
+ENGLISH_NOTES = '[CDEFGAB]'
+GERMAN_NOTES = '[CDEFGAH]'
+NEOLATIN_NOTES = '(Do|Re|Mi|Fa|Sol|La|Si)'
+CHORD_SUFFIXES = '(b|bb)?(#)?(m|maj7|maj|min7|min|sus)?(1|2|3|4|5|6|7|8|9)?'
 SLIM_CHARS = 'fiíIÍjlĺľrtť.,;/ ()|"\'!:\\'
-CHORD_LINE_MATCH = re.compile(r'\[(.*?)\]([\u0080-\uFFFF,\w]*)'
-                              r'([\u0080-\uFFFF\w\s\.\,\!\?\;\:\|\"\'\-\_]*)(\Z)?')
 CHORD_TEMPLATE = '<span class="chordline">{chord}</span>'
 FIRST_CHORD_TEMPLATE = '<span class="chordline firstchordline">{chord}</span>'
 CHORD_LINE_TEMPLATE = '<span class="chord"><span><strong>{chord}</strong></span></span>{tail}{whitespace}{remainder}'
 WHITESPACE_TEMPLATE = '<span class="ws">{whitespaces}</span>'
-
 VERSE = 'The Lord said to {r}Noah{/r}: \n' \
     'There\'s gonna be a {su}floody{/su}, {sb}floody{/sb}\n' \
     'The Lord said to {g}Noah{/g}:\n' \
@@ -66,6 +67,72 @@ AUTHOR = 'John Doe'
 FOOTER_COPYRIGHT = 'Public Domain'
 CCLI_NO = '123456'
 
+# Just so we can cache the regular expression objects
+_chord_cache = {}
+_line_cache = {}
+
+
+def _construct_chord_regex(notes):
+    """
+    Create the regex for a particular set of notes
+
+    :param notes: The regular expression for a set of valid notes
+    :return: An expanded regular expression for valid chords
+    """
+    chord = notes + CHORD_SUFFIXES
+    return '(' + chord + '(/' + chord + ')?)'
+
+
+def _construct_chord_match(notes):
+    """
+    Construct chord matching regular expression object
+
+    :param notes: The regular expression for a set of valid notes
+    :return: A compiled regular expression object
+    """
+    return re.compile(r'\[' + _construct_chord_regex(notes) + r'\]')
+
+
+def _construct_chord_line_match(notes):
+    """
+    Construct a chord line matching regular expression object
+
+    :param notes: The regular expression for a set of valid notes
+    :return: A compiled regular expression object
+    """
+    return re.compile(r'\[' + _construct_chord_regex(notes) + r'\]([\u0080-\uFFFF,\w]*)'
+                      r'([\u0080-\uFFFF\w\s\.\,\!\?\;\:\|\"\'\-\_]*)(\Z)?')
+
+
+def _get_chord_match():
+    """
+    Get the right chord_match object based on the current chord notation
+    """
+    notation = Registry().get('settings').value('songs/chord notation')
+    if notation not in _chord_cache.keys():
+        if notation == 'german':
+            _chord_cache[notation] = _construct_chord_match(GERMAN_NOTES)
+        elif notation == 'neo-latin':
+            _chord_cache[notation] = _construct_chord_match(NEOLATIN_NOTES)
+        else:
+            _chord_cache[notation] = _construct_chord_match(ENGLISH_NOTES)
+    return _chord_cache[notation]
+
+
+def _get_line_match():
+    """
+    Get the right chold line match based on the current chord notation
+    """
+    notation = Registry().get('settings').value('songs/chord notation')
+    if notation not in _line_cache.keys():
+        if notation == 'german':
+            _line_cache[notation] = _construct_chord_line_match(GERMAN_NOTES)
+        elif notation == 'neo-latin':
+            _line_cache[notation] = _construct_chord_line_match(NEOLATIN_NOTES)
+        else:
+            _line_cache[notation] = _construct_chord_line_match(ENGLISH_NOTES)
+    return _line_cache[notation]
+
 
 def remove_chords(text):
     """
@@ -73,7 +140,7 @@ def remove_chords(text):
 
     :param text: Text to be cleaned
     """
-    return re.sub(r'\[.+?\]', r'', text)
+    return _get_chord_match().sub(r'', text)
 
 
 def remove_tags(text, can_remove_chords=False):
@@ -120,11 +187,11 @@ def render_chords_in_line(match):
     # The actual chord, would be "G" in match "[G]sweet the "
     chord = match.group(1)
     # The tailing word of the chord, would be "sweet" in match "[G]sweet the "
-    tail = match.group(2)
+    tail = match.group(11)
     # The remainder of the line, until line end or next chord. Would be " the " in match "[G]sweet the "
-    remainder = match.group(3)
+    remainder = match.group(12)
     # Line end if found, else None
-    end = match.group(4)
+    end = match.group(13)
     # Based on char width calculate width of chord
     for chord_char in chord:
         if chord_char not in SLIM_CHARS:
@@ -186,6 +253,7 @@ def render_chords(text):
     :returns str: The text containing the rendered chords
     """
     text_lines = text.split('{br}')
+    chord_line_match = _get_line_match()
     rendered_lines = []
     chords_on_prev_line = False
     for line in text_lines:
@@ -197,7 +265,7 @@ def render_chords(text):
                 chord_template = FIRST_CHORD_TEMPLATE
                 chords_on_prev_line = True
             # Matches a chord, a tail, a remainder and a line end. See expand_and_align_chords_in_line() for more info.
-            new_line = chord_template.format(chord=CHORD_LINE_MATCH.sub(render_chords_in_line, line))
+            new_line = chord_template.format(chord=chord_line_match.sub(render_chords_in_line, line))
             rendered_lines.append(new_line)
         else:
             chords_on_prev_line = False
@@ -533,7 +601,7 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
         if item and item.is_capable(ItemCapabilities.NoLineBreaks):
             line_end = ' '
         # Bibles
-        if item and item.is_capable(ItemCapabilities.CanWordSplit):
+        if item and item.name == 'bibles':
             pages = self._paginate_slide_words(text.split('\n'), line_end)
         # Songs and Custom
         elif item is None or (item and item.is_capable(ItemCapabilities.CanSoftBreak)):
@@ -549,9 +617,9 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
                     text = text.replace(' [---]', '[---]')
                 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:
+                # Grab the number of occurrences of "[---]" in the text to use as a max number of loops
+                count = text.count('[---]')
+                for _ in range(count):
                     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).

tests/functional/openlp_core/api/endpoint/test_controller.py

@@ -75,6 +75,7 @@ class TestController(TestCase):
         self.mocked_renderer.format_slide = self.mocked_slide_formater
         Registry().register('live_controller', self.mocked_live_controller)
         Registry().register('renderer', self.mocked_renderer)
+        Registry().register('settings', MagicMock(**{'value.return_value': 'english'}))
 
     def test_controller_text_empty(self):
         """

tests/functional/openlp_core/display/test_render.py

@@ -52,11 +52,12 @@ def test_remove_tags(mocked_get_tags):
 
 
 @patch('openlp.core.display.render.FormattingTags.get_html_tags')
-def test_render_tags(mocked_get_tags):
+def test_render_tags(mocked_get_tags, settings):
     """
     Test the render_tags() method.
     """
     # GIVEN: Mocked get_html_tags() method.
+    settings.setValue('songs/chord notation', 'english')
     mocked_get_tags.return_value = [
         {
             'desc': 'Black',
@@ -91,12 +92,13 @@ def test_render_tags(mocked_get_tags):
     assert result_string == expected_string, 'The strings should be identical.'
 
 
-def test_render_chords():
+def test_render_chords(settings):
     """
     Test that the rendering of chords works as expected.
     """
     # GIVEN: A lyrics-line with chords
-    text_with_chords = 'H[C]alleluya.[F] [G]'
+    settings.setValue('songs/chord notation', 'english')
+    text_with_chords = 'H[C]alleluya.[F] [G/B]'
 
     # WHEN: Expanding the chords
     text_with_rendered_chords = render_chords(text_with_chords)
@@ -104,15 +106,16 @@ def test_render_chords():
     # THEN: We should get html that looks like below
     expected_html = '<span class="chordline firstchordline">H<span class="chord"><span><strong>C</strong></span>' \
                     '</span>alleluya.<span class="chord"><span><strong>F</strong></span></span><span class="ws">' \
-                    '&nbsp;&nbsp;</span> <span class="chord"><span><strong>G</strong></span></span></span>'
+                    '&nbsp;&nbsp;</span> <span class="chord"><span><strong>G/B</strong></span></span></span>'
     assert text_with_rendered_chords == expected_html, 'The rendered chords should look as expected'
 
 
-def test_render_chords_with_special_chars():
+def test_render_chords_with_special_chars(settings):
     """
     Test that the rendering of chords works as expected when special chars are involved.
     """
     # GIVEN: A lyrics-line with chords
+    settings.setValue('songs/chord notation', 'english')
     text_with_chords = "I[D]'M NOT MOVED BY WHAT I SEE HALLE[F]LUJA[C]H"
 
     # WHEN: Expanding the chords
@@ -155,11 +158,12 @@ def test_compare_chord_lyric_long_chord():
     assert ret == 4, 'The returned value should 4 because the chord is longer than the lyric'
 
 
-def test_render_chords_for_printing():
+def test_render_chords_for_printing(settings):
     """
     Test that the rendering of chords for printing works as expected.
     """
     # GIVEN: A lyrics-line with chords
+    settings.setValue('songs/chord notation', 'english')
     text_with_chords = '{st}[D]Amazing {r}gr[D7]ace{/r}  how [G]sweet the [D]sound  [F]{/st}'
     FormattingTags.load_tags()
 

tests/functional/openlp_core/lib/test_serviceitem.py

@@ -75,11 +75,12 @@ class TestServiceItem(TestCase, TestMixin):
         Set up the Registry
         """
         self.build_settings()
-        Settings().extend_default_settings(__default_settings__)
+        self.setting.extend_default_settings(__default_settings__)
+        self.setting.setValue('songs/chord notation', 'english')
         Registry.create()
+        Registry().register('settings', self.setting)
         # Mock the renderer and its format_slide method
         mocked_renderer = MagicMock()
-        Registry().register('settings', Settings())
 
         def side_effect_return_arg(arg1, arg2):
             return [arg1]

tests/interfaces/openlp_plugins/planningcenter/lib/test_songimport.py

@@ -23,7 +23,7 @@ Package to test the openlp.plugins.planningcenter.lib.songimport package.
 """
 import datetime
 from unittest import TestCase
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 from openlp.core.common.registry import Registry
 from openlp.plugins.planningcenter.lib.songimport import PlanningCenterSongImport
@@ -38,16 +38,18 @@ class TestSongImport(TestCase, TestMixin):
         """
         Create the class
         """
-        self.registry = Registry()
+        mocked_settings = MagicMock()
+        mocked_settings.value.return_value = 'english'
         Registry.create()
-        with patch('openlp.core.common.registry.Registry.get'):
-            self.song_import = PlanningCenterSongImport()
+        self.registry = Registry()
+        self.registry.register('settings', mocked_settings)
+        self.registry.register('songs', MagicMock())
+        self.song_import = PlanningCenterSongImport()
 
     def tearDown(self):
         """
         Delete all the C++ objects at the end so that we don't have a segfault
         """
-        del self.registry
         del self.song_import
 
     def test_add_song_without_lyrics(self):