From b85f1db06f95fdaafca67c68b6b876b4fa734f4b Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 15 Jan 2020 21:57:56 -0700 Subject: [PATCH] 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 | 92 ++++++++++++++++--- .../api/endpoint/test_controller.py | 1 + .../openlp_core/display/test_render.py | 16 ++-- .../openlp_core/lib/test_serviceitem.py | 5 +- .../planningcenter/lib/test_songimport.py | 12 ++- 5 files changed, 101 insertions(+), 25 deletions(-) diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index 4bfa129fd..ea3f41b83 100644 --- a/openlp/core/display/render.py +++ b/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 = '{chord}' FIRST_CHORD_TEMPLATE = '{chord}' CHORD_LINE_TEMPLATE = '{chord}{tail}{whitespace}{remainder}' WHITESPACE_TEMPLATE = '{whitespaces}' - 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). diff --git a/tests/functional/openlp_core/api/endpoint/test_controller.py b/tests/functional/openlp_core/api/endpoint/test_controller.py index 09ed9ca1a..b27fd9fec 100644 --- a/tests/functional/openlp_core/api/endpoint/test_controller.py +++ b/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): """ diff --git a/tests/functional/openlp_core/display/test_render.py b/tests/functional/openlp_core/display/test_render.py index 094d2ec0d..0e3c30e78 100644 --- a/tests/functional/openlp_core/display/test_render.py +++ b/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 = 'HC' \ 'alleluya.F' \ - '   G' + '   G/B' 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() diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py index 66d187d27..d56283c36 100644 --- a/tests/functional/openlp_core/lib/test_serviceitem.py +++ b/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] diff --git a/tests/interfaces/openlp_plugins/planningcenter/lib/test_songimport.py b/tests/interfaces/openlp_plugins/planningcenter/lib/test_songimport.py index de0aa38e2..05df4d972 100644 --- a/tests/interfaces/openlp_plugins/planningcenter/lib/test_songimport.py +++ b/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):