Fixed various bugs in the songbeamer chord import, and added a test.

openlp/plugins/songs/lib/importers/songbeamer.py

@@ -116,12 +116,12 @@ class SongBeamerImport(SongImport):
             file_name = os.path.split(import_file)[1]
             if os.path.isfile(import_file):
                 # Detect the encoding
-                self.input_file_encoding = get_file_encoding(file_name)['encoding']
+                self.input_file_encoding = get_file_encoding(import_file)['encoding']
                 # The encoding should only be ANSI (cp1251), UTF-8, Unicode, Big-Endian-Unicode.
-                # So if it doesn't start with 'u' we default to cp1251. See:
+                # So if it doesn't start with 'u' we default to cp1252. See:
                 # https://forum.songbeamer.com/viewtopic.php?p=419&sid=ca4814924e37c11e4438b7272a98b6f2
-                if self.input_file_encoding.lower().startswith('u'):
+                if not self.input_file_encoding.lower().startswith('u'):
-                    self.input_file_encoding = 'cp1251'
+                    self.input_file_encoding = 'cp1252'
                 infile = open(import_file, 'rt', encoding=self.input_file_encoding)
                 song_data = infile.readlines()
                 infile.close()
@@ -133,9 +133,10 @@ class SongBeamerImport(SongImport):
             line_number = -1
             for line in song_data:
                 line = line.rstrip()
+                stripped_line = line.strip()
                 if line.startswith('#') and not read_verses:
                     self.parse_tags(line)
-                elif line.startswith('---'):
+                elif stripped_line.startswith('---'):
                     # '---' is a verse breaker
                     if self.current_verse:
                         self.replace_html_tags()
@@ -150,19 +151,19 @@ class SongBeamerImport(SongImport):
                         if first_line:
                             self.current_verse = first_line.strip() + '\n'
                     line_number += 1
-                elif line.startswith('--'):
+                elif stripped_line.startswith('--'):
                     # '--' is a page breaker, we convert to optional page break
                     self.current_verse += '[---]\n'
                     line_number += 1
                 elif read_verses:
-                    line = self.insert_chords(line_number, line)
                     if verse_start:
                         verse_start = False
                         if not self.check_verse_marks(line):
                             self.current_verse += line.strip() + '\n'
                     else:
+                        line = self.insert_chords(line_number, line)
                         self.current_verse += line.strip() + '\n'
-                    line_number += 1
+                        line_number += 1
             if self.current_verse:
                 self.replace_html_tags()
                 self.add_verse(self.current_verse, self.current_verse_type)

tests/functional/openlp_plugins/songs/test_songbeamerimport.py

@@ -42,10 +42,17 @@ class TestSongBeamerFileImport(SongImportTestHelper):
         self.importer_module_name = 'songbeamer'
         super(TestSongBeamerFileImport, self).__init__(*args, **kwargs)
 
-    def test_song_import(self):
+    @patch('openlp.plugins.songs.lib.importers.songbeamer.Settings')
+    def test_song_import(self, mocked_settings):
         """
         Test that loading an OpenSong file works correctly on various files
         """
+        # Mock out the settings - always return False
+        mocked_returned_settings = MagicMock()
+        mocked_returned_settings.value.return_value = False
+        mocked_settings.return_value = mocked_returned_settings
+        self.file_import([os.path.join(TEST_PATH, 'Amazing Grace.sng')],
+                         self.load_external_result_data(os.path.join(TEST_PATH, 'Amazing Grace.json')))
         self.file_import([os.path.join(TEST_PATH, 'Lobsinget dem Herrn.sng')],
                          self.load_external_result_data(os.path.join(TEST_PATH, 'Lobsinget dem Herrn.json')))
 
@@ -59,6 +66,16 @@ class TestSongBeamerImport(TestCase):
         Create the registry
         """
         Registry.create()
+        self.song_import_patcher = patch('openlp.plugins.songs.lib.importers.songbeamer.SongImport')
+        self.song_import_patcher.start()
+        mocked_manager = MagicMock()
+        self.importer = SongBeamerImport(mocked_manager, filenames=[])
+
+    def tearDown(self):
+        """
+        Clean up
+        """
+        self.song_import_patcher.stop()
 
     def test_create_importer(self):
         """
@@ -78,43 +95,37 @@ class TestSongBeamerImport(TestCase):
         """
         Test SongBeamerImport.do_import handles different invalid import_source values
         """
-        # GIVEN: A mocked out SongImport class, and a mocked out "manager"
+        # GIVEN: A mocked out import wizard
-        with patch('openlp.plugins.songs.lib.importers.songbeamer.SongImport'):
+        mocked_import_wizard = MagicMock()
-            mocked_manager = MagicMock()
+        self.importer.import_wizard = mocked_import_wizard
-            mocked_import_wizard = MagicMock()
+        self.importer.stop_import_flag = True
-            importer = SongBeamerImport(mocked_manager, filenames=[])
-            importer.import_wizard = mocked_import_wizard
-            importer.stop_import_flag = True
 
-            # WHEN: Import source is not a list
+        # WHEN: Import source is not a list
-            for source in ['not a list', 0]:
+        for source in ['not a list', 0]:
-                importer.import_source = source
+            self.importer.import_source = source
 
-                # THEN: do_import should return none and the progress bar maximum should not be set.
+            # THEN: do_import should return none and the progress bar maximum should not be set.
-                self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list')
+            self.assertIsNone(self.importer.do_import(), 'do_import should return None when import_source is not a list')
-                self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
+            self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
-                                 'setMaxium on import_wizard.progress_bar should not have been called')
+                             'setMaxium on import_wizard.progress_bar should not have been called')
 
     def test_valid_import_source(self):
         """
         Test SongBeamerImport.do_import handles different invalid import_source values
         """
-        # GIVEN: A mocked out SongImport class, and a mocked out "manager"
+        # GIVEN: A mocked out import wizard
-        with patch('openlp.plugins.songs.lib.importers.songbeamer.SongImport'):
+        mocked_import_wizard = MagicMock()
-            mocked_manager = MagicMock()
+        self.importer.import_wizard = mocked_import_wizard
-            mocked_import_wizard = MagicMock()
+        self.importer.stop_import_flag = True
-            importer = SongBeamerImport(mocked_manager, filenames=[])
-            importer.import_wizard = mocked_import_wizard
-            importer.stop_import_flag = True
 
-            # WHEN: Import source is a list
+        # WHEN: Import source is a list
-            importer.import_source = ['List', 'of', 'files']
+        self.importer.import_source = ['List', 'of', 'files']
 
-            # THEN: do_import should return none and the progress bar setMaximum should be called with the length of
+        # THEN: do_import should return none and the progress bar setMaximum should be called with the length of
-            #       import_source.
+        #       import_source.
-            self.assertIsNone(importer.do_import(),
+        self.assertIsNone(self.importer.do_import(),
-                              'do_import should return None when import_source is a list and stop_import_flag is True')
+                          'do_import should return None when import_source is a list and stop_import_flag is True')
-            mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source))
+        mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(self.importer.import_source))
 
     def test_check_verse_marks(self):
         """
@@ -123,75 +134,75 @@ class TestSongBeamerImport(TestCase):
 
         # GIVEN: line with unnumbered verse-type
         line = 'Refrain'
-        self.current_verse_type = None
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back true and c as self.current_verse_type
+        # THEN: we should get back true and c as self.importer.current_verse_type
         self.assertTrue(result, 'Versemark for <Refrain> should be found, value true')
-        self.assertEqual(self.current_verse_type, 'c', '<Refrain> should be interpreted as <c>')
+        self.assertEqual(self.importer.current_verse_type, 'c', '<Refrain> should be interpreted as <c>')
 
         # GIVEN: line with unnumbered verse-type and trailing space
         line = 'ReFrain '
-        self.current_verse_type = None
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back true and c as self.current_verse_type
+        # THEN: we should get back true and c as self.importer.current_verse_type
         self.assertTrue(result, 'Versemark for <ReFrain > should be found, value true')
-        self.assertEqual(self.current_verse_type, 'c', '<ReFrain > should be interpreted as <c>')
+        self.assertEqual(self.importer.current_verse_type, 'c', '<ReFrain > should be interpreted as <c>')
 
         # GIVEN: line with numbered verse-type
         line = 'VersE 1'
-        self.current_verse_type = None
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back true and v1 as self.current_verse_type
+        # THEN: we should get back true and v1 as self.importer.current_verse_type
         self.assertTrue(result, 'Versemark for <VersE 1> should be found, value true')
-        self.assertEqual(self.current_verse_type, 'v1', u'<VersE 1> should be interpreted as <v1>')
+        self.assertEqual(self.importer.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
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back true and o as self.current_verse_type
+        # THEN: we should get back true and o as self.importer.current_verse_type
         self.assertTrue(result, 'Versemark for <$$M=special> should be found, value true')
-        self.assertEqual(self.current_verse_type, 'o', u'<$$M=special> should be interpreted as <o>')
+        self.assertEqual(self.importer.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
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back false and none as self.current_verse_type
+        # THEN: we should get back false and none as self.importer.current_verse_type
         self.assertFalse(result, 'No versemark for <Jesus my saviour> should be found, value false')
-        self.assertIsNone(self.current_verse_type, '<Jesus my saviour> should be interpreted as none versemark')
+        self.assertIsNone(self.importer.current_verse_type, '<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
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back false and none as self.current_verse_type
+        # THEN: we should get back false and none as self.importer.current_verse_type
         self.assertFalse(result, 'No versemark for <Praise him> should be found, value false')
-        self.assertIsNone(self.current_verse_type, '<Praise him> should be interpreted as none versemark')
+        self.assertIsNone(self.importer.current_verse_type, '<Praise him> should be interpreted as none versemark')
 
         # GIVEN: line with only a space (could occur, nothing regular)
         line = ' '
-        self.current_verse_type = None
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back false and none as self.current_verse_type
+        # THEN: we should get back false and none as self.importer.current_verse_type
         self.assertFalse(result, 'No versemark for < > should be found, value false')
-        self.assertIsNone(self.current_verse_type, '< > should be interpreted as none versemark')
+        self.assertIsNone(self.importer.current_verse_type, '< > should be interpreted as none versemark')
 
         # GIVEN: blank line (could occur, nothing regular)
         line = ''
-        self.current_verse_type = None
+        self.importer.current_verse_type = None
         # WHEN: line is being checked for verse marks
-        result = SongBeamerImport.check_verse_marks(self, line)
+        result = self.importer.check_verse_marks(line)
-        # THEN: we should get back false and none as self.current_verse_type
+        # THEN: we should get back false and none as self.importer.current_verse_type
         self.assertFalse(result, 'No versemark for <> should be found, value false')
-        self.assertIsNone(self.current_verse_type, '<> should be interpreted as none versemark')
+        self.assertIsNone(self.importer.current_verse_type, '<> should be interpreted as none versemark')
 
     def test_verse_marks_defined_in_lowercase(self):
         """

tests/resources/songbeamersongs/Amazing Grace.json

@@ -0,0 +1,29 @@
+{
+    "authors": [
+        "John Newton"
+    ],
+    "title": "Amazing grace",
+    "verse_order_list": ["v1", "v2", "v3", "v4", "v5"],
+    "verses": [
+        [
+            "[D]Amazing gr[D7]ace how [G]sweet the [D]sound\nThat saved a [E7]wretch like [A7]me\nI [D]once was [D7]lost but [G]now im [D]found\nWas b[E7]lind but no[A7]w i [D]see [A7]\n",
+            "v1"
+        ],
+        [
+            "T'was [D]grace that [D7]taught my [G]heart to [D]fear\nAnd grace my [E7]fears [A7]relieved\nHow [D]precious [D7]did that [G]Grace [D]appear\nThe [E7]hour I [A7]first bel[D]ieved.[A7]\n",
+            "v2"
+        ],
+        [
+            "Through [D]many [D7]dangers, [G]toils and [D]snares\nI have [E7]already [A7]come;\n'Tis [D]Grace that [D7]brought me [G]safe thus [D]far\nand [E7]Grace will [A7]lead me [D]home. [A7]\n",
+            "v3"
+        ],
+        [
+            "When we[D]'ve been here[D7] ten thous[G]and y[D]ears\nBright shining [E7]as the [A7]sun.\nWe've [D]no less [D7]days to s[G]ing God's p[D]raise\nThan w[E7]hen we've [A7]first [D]begun.[A7]\n",
+            "v4"
+        ],
+        [
+            "[D]Amazing gr[D7]ace how [G]sweet the [D]sound\nThat saved a [E7]wretch like [A7]me\nI [D]once was [D7]lost but [G]now im [D]found\nWas b[E7]lind but no[A7]w i [D]see [A]\n",
+            "v5"
+        ]
+    ]
+}

tests/resources/songbeamersongs/Amazing Grace.sng

@@ -0,0 +1,37 @@
+#LangCount=1
+#Title=Amazing grace
+#Chords=MCwwLEQNMTAsMCxENw0xOCwwLEcNMjgsMCxEDTEzLDEsRTcNMjUsMSxBNw0yLDIsRA0xMSwyLEQ3DTIwLDIsRw0yNywyLEQNNSwzLEU3DTE2LDMsQTcNMjAsMyxEDTI0LDMsQTcNNiw1LEQNMTcsNSxENw0yNyw1LEcNMzYsNSxEDTEzLDYsRTcNMTksNixBNw00LDcsRA0xMyw3LEQ3DTIyLDcsRw0yOCw3LEQNNCw4LEU3DTExLDgsQTcNMjAsOCxEDTI2LDgsQTcNOCwxMCxEDTEzLDEwLEQ3DTIyLDEwLEcNMzIsMTAsRA03LDExLEU3DTE1LDExLEE3DTUsMTIsRA0xNiwxMixENw0yNywxMixHDTM3LDEyLEQNNCwxMyxFNw0xNSwxMyxBNw0yMywxMyxEDTI5LDEzLEE3DTcsMTUsRA0yMCwxNSxENw0zMCwxNSxHDTM1LDE1LEQNMTUsMTYsRTcNMjIsMTYsQTcNNiwxNyxEDTE0LDE3LEQ3DTIzLDE3LEcNMzQsMTcsRA02LDE4LEU3DTE2LDE4LEE3DTIyLDE4LEQNMjgsMTgsQTcNMCwyMCxEDTEwLDIwLEQ3DTE4LDIwLEcNMjgsMjAsRA0xMywyMSxFNw0yNSwyMSxBNw0yLDIyLEQNMTEsMjIsRDcNMjAsMjIsRw0yNywyMixEDTUsMjMsRTcNMTYsMjMsQTcNMjAsMjMsRA0yNCwyMyxBDQ==
+#Author=John Newton
+#Editor=SongBeamer 4.37a
+#Version=3
+#VerseOrder=Verse 1,Verse 2,Verse 3,Verse 4,Verse 5
+---
+Verse 1
+Amazing grace how sweet the sound
+That saved a wretch like me
+I once was lost but now im found
+Was blind but now i see
+---
+Verse 2
+T'was grace that taught my heart to fear
+And grace my fears relieved
+How precious did that Grace appear
+The hour I first believed.
+---
+Verse 3
+Through many dangers, toils and snares
+I have already come;
+'Tis Grace that brought me safe thus far
+and Grace will lead me home.
+---
+Verse 4
+When we've been here ten thousand years
+Bright shining as the sun.
+We've no less days to sing God's praise
+Than when we've first begun.
+---
+Verse 5
+Amazing grace how sweet the sound
+That saved a wretch like me
+I once was lost but now im found
+Was blind but now i see