openlp-core team mailing list archive

[Stefan Strasser <strada@xxxxxxxxxxx>]

Stefan Strasser has proposed merging lp:~strada/openlp/songbeamer-import-enhancements into lp:openlp.

Commit message:
Songbeamer-import: additional verse-marks to be recognized (Misc,Part,Teil,$$M=); accepts now also -- as page-break; added functional test for check_verse_marks

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~strada/openlp/songbeamer-import-enhancements/+merge/215578

Some enhancements for songbeamer-import:
- added some missing verse-marks to be recognized by the importer (Misc,Part,Teil,$$M=); all of them set to be translated to VerseType.Other (I would prefer a special marker for Part/Teil but this is currently not existing in openlp)
- special handling for mark $$M= in check_verse_marks because the name of this mark may be different (after the =) and it is not countable; this mark is in songbeamer used for custom marks (on importing from other formats or manual)
- page-break-recognition now accepts also -- (in songbeamer -- and --- are both identically used to separate slides/verses, the only difference is in the layout of a printout)
- added a functional test for the function check_verse_marks (checking the recognition of different possible lines in one test)
-- 
https://code.launchpad.net/~strada/openlp/songbeamer-import-enhancements/+merge/215578
Your team OpenLP Core is requested to review the proposed merge of lp:~strada/openlp/songbeamer-import-enhancements into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/songbeamerimport.py'
--- openlp/plugins/songs/lib/songbeamerimport.py	2014-03-29 13:31:28 +0000
+++ openlp/plugins/songs/lib/songbeamerimport.py	2014-04-13 17:50:47 +0000
@@ -56,11 +56,15 @@
         'Zwischenspiel': VerseType.tags[VerseType.Bridge],
         'Pre-Chorus': VerseType.tags[VerseType.PreChorus],
         'Pre-Refrain': VerseType.tags[VerseType.PreChorus],
+        'Misc': VerseType.tags[VerseType.Other],
         'Pre-Bridge': VerseType.tags[VerseType.Other],
         'Pre-Coda': VerseType.tags[VerseType.Other],
+        'Part': VerseType.tags[VerseType.Other],
+        'Teil': VerseType.tags[VerseType.Other],
         'Unbekannt': VerseType.tags[VerseType.Other],
         'Unknown': VerseType.tags[VerseType.Other],
-        'Unbenannt': VerseType.tags[VerseType.Other]
+        'Unbenannt': VerseType.tags[VerseType.Other],
+        '$$M=': VerseType.tags[VerseType.Other]
     }
 
 
@@ -132,7 +136,8 @@
                 line = str(line).strip()
                 if line.startswith('#') and not read_verses:
                     self.parseTags(line)
-                elif line.startswith('---'):
+                elif line.startswith('--'):
+                # --- and -- allowed for page-breaks (difference in Songbeamer only in printout)
                     if self.current_verse:
                         self.replace_html_tags()
                         self.add_verse(self.current_verse, self.current_verse_type)
@@ -282,4 +287,7 @@
                 if marks[1].isdigit():
                     self.current_verse_type += marks[1]
             return True
+        elif marks[0].startswith('$$M='):  # this verse-mark cannot be numbered
+            self.current_verse_type = SongBeamerTypes.MarkTypes['$$M=']
+            return True
         return False

=== modified file 'tests/functional/openlp_plugins/songs/test_songbeamerimport.py'
--- tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py	2014-04-13 17:50:47 +0000
@@ -35,6 +35,7 @@
 
 from tests.functional import MagicMock, patch
 from openlp.plugins.songs.lib.songbeamerimport import SongBeamerImport
+from openlp.plugins.songs.lib import VerseType
 
 TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
                                          '..', '..', '..', 'resources', 'songbeamersongs'))
@@ -153,3 +154,80 @@
                     self.assertEquals(importer.song_number, song_number, 'song_number for %s should be %s' %
                                                                          (song_file, song_number))
                 mocked_finish.assert_called_with()
+
+    def check_verse_marks_test(self):
+        """
+        Tests different lines to see if a verse mark is detected or not
+        """
+
+        # GIVEN: line with unnumbered verse-type
+        line = 'Refrain'
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back true and c as self.current_verse_type
+        assert result is True, u'Versemark for <Refrain> should be found, value true'
+        assert self.current_verse_type == 'c', u'<Refrain> should be interpreted as <c>'
+
+        # GIVEN: line with unnumbered verse-type and trailing space
+        line = 'Refrain '
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back true and c as self.current_verse_type
+        assert result is True, u'Versemark for <Refrain > should be found, value true'
+        assert self.current_verse_type == 'c', u'<Refrain > should be interpreted as <c>'
+
+        # GIVEN: line with numbered verse-type
+        line = 'Verse 1'
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back true and v1 as self.current_verse_type
+        assert result is True, u'Versemark for <Verse 1> should be found, value true'
+        assert self.current_verse_type == 'v1', u'<Verse 1> should be interpreted as <v1>'
+
+        # GIVEN: line with special unnumbered verse-mark (used in Songbeamer to allow usage of non-supported tags)
+        line = '$$M=special'
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back true and o as self.current_verse_type
+        assert result is True, u'Versemark for <$$M=special> should be found, value true'
+        assert self.current_verse_type == 'o', u'<$$M=special> should be interpreted as <o>'
+
+        # GIVEN: line with song-text with 3 words
+        line = 'Jesus my saviour'
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back false and none as self.current_verse_type
+        assert result is False, u'No versemark for <Jesus my saviour> should be found, value false'
+        assert self.current_verse_type is None, u'<Jesus my saviour> should be interpreted as none versemark'
+
+        # GIVEN: line with song-text with 2 words
+        line = 'Praise him'
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back false and none as self.current_verse_type
+        assert result is False, u'No versemark for <Praise him> should be found, value false'
+        assert self.current_verse_type is None, u'<Praise him> should be interpreted as none versemark'
+
+        # GIVEN: line with only a space (could occur, nothing regular)
+        line = ' '
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back false and none as self.current_verse_type
+        assert result is False, u'No versemark for < > should be found, value false'
+        assert self.current_verse_type is None, u'< > should be interpreted as none versemark'
+
+        # GIVEN: blank line (could occur, nothing regular)
+        line = ''
+        self.current_verse_type = None
+        # WHEN: line is being checked for verse marks
+        result = SongBeamerImport.check_verse_marks(self, line)
+        # THEN: we should get back false and none as self.current_verse_type
+        assert result is False, u'No versemark for <> should be found, value false'
+        assert self.current_verse_type is None, u'<> should be interpreted as none versemark'