diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py
index dc198d4b7..c15ce0b73 100644
--- a/openlp/plugins/songs/lib/__init__.py
+++ b/openlp/plugins/songs/lib/__init__.py
@@ -278,7 +278,7 @@ class VerseType(object):
if verse_index is None:
verse_index = VerseType.from_string(verse_name, default)
elif len(verse_name) == 1:
- verse_index = VerseType.from_translated_tag(verse_name, None)
+ verse_index = VerseType.from_translated_tag(verse_name, default)
if verse_index is None:
verse_index = VerseType.from_tag(verse_name, default)
else:
diff --git a/openlp/plugins/songs/lib/opensongimport.py b/openlp/plugins/songs/lib/opensongimport.py
index b418e1d37..1563f23d1 100644
--- a/openlp/plugins/songs/lib/opensongimport.py
+++ b/openlp/plugins/songs/lib/opensongimport.py
@@ -162,8 +162,7 @@ class OpenSongImport(SongImport):
if attr in fields:
ustring = str(root.__getattr__(attr))
if isinstance(fn_or_string, str):
- match = re.match('(\D*)(\d+.*)', ustring)
- if match:
+ if attr in ['ccli']:
setattr(self, fn_or_string, int(ustring))
else:
setattr(self, fn_or_string, ustring)
@@ -261,7 +260,14 @@ class OpenSongImport(SongImport):
verse_def = '%s%s' % (verse_tag, verse_num[:length])
verse_joints[verse_def] = '%s\n[---]\n%s' % (verse_joints[verse_def], lines) \
if verse_def in verse_joints else lines
- for verse_def, lines in verse_joints.items():
+ # Parsing the dictionary produces the elements in a non-intuitive order. While it "works", it's not a
+ # natural layout should the user come back to edit the song. Instead we sort by the verse type, so that we
+ # get all the verses in order (v1, v2, ...), then the chorus(es), bridge(s), pre-chorus(es) etc. We use a
+ # tuple for the key, since tuples naturally sort in this manner.
+ verse_defs = sorted(verse_joints.keys(),
+ key=lambda verse_def: (VerseType.from_tag(verse_def[0]), int(verse_def[1:])))
+ for verse_def in verse_defs:
+ lines = verse_joints[verse_def]
self.add_verse(lines, verse_def)
if not self.verses:
self.add_verse('')
@@ -282,6 +288,8 @@ class OpenSongImport(SongImport):
# Assume it's no.1 if there are no digits
verse_tag = verse_def
verse_num = '1'
+ verse_index = VerseType.from_loose_input(verse_tag)
+ verse_tag = VerseType.tags[verse_index]
verse_def = '%s%s' % (verse_tag, verse_num)
if verse_num in verses.get(verse_tag, {}):
self.verse_order_list.append(verse_def)
diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py
index 93e0b508d..53310d258 100644
--- a/openlp/plugins/songs/lib/songimport.py
+++ b/openlp/plugins/songs/lib/songimport.py
@@ -202,7 +202,7 @@ class SongImport(QtCore.QObject):
# Book name:'NRH' and
# Song number: 231
book_and_number = book_and_number.strip()
- if book_and_number == '':
+ if not book_and_number:
return
book_and_number = book_and_number.replace('No.', ' ')
if ' ' in book_and_number:
@@ -233,7 +233,7 @@ class SongImport(QtCore.QObject):
"""
if self.comments.find(comment) >= 0:
return
- if comment != '':
+ if comment:
self.comments += comment.strip() + '\n'
def add_copyright(self, copyright):
@@ -242,7 +242,7 @@ class SongImport(QtCore.QObject):
"""
if self.copyright.find(copyright) >= 0:
return
- if self.copyright != '':
+ if self.copyright:
self.copyright += ' '
self.copyright += copyright
diff --git a/tests/functional/openlp_plugins/songs/test_opensongimport.py b/tests/functional/openlp_plugins/songs/test_opensongimport.py
index a63d09068..70d3b342a 100644
--- a/tests/functional/openlp_plugins/songs/test_opensongimport.py
+++ b/tests/functional/openlp_plugins/songs/test_opensongimport.py
@@ -56,6 +56,8 @@ class TestOpenSongFileImport(SongImportTestHelper):
self.load_external_result_data(os.path.join(TEST_PATH, 'Amazing Grace.json')))
self.file_import(os.path.join(TEST_PATH, 'Beautiful Garden Of Prayer'),
self.load_external_result_data(os.path.join(TEST_PATH, 'Beautiful Garden Of Prayer.json')))
+ self.file_import(os.path.join(TEST_PATH, 'One, Two, Three, Four, Five'),
+ self.load_external_result_data(os.path.join(TEST_PATH, 'One, Two, Three, Four, Five.json')))
class TestOpenSongImport(TestCase):
diff --git a/tests/helpers/songfileimport.py b/tests/helpers/songfileimport.py
index 009a2c92a..36beef6e5 100644
--- a/tests/helpers/songfileimport.py
+++ b/tests/helpers/songfileimport.py
@@ -33,7 +33,7 @@ song files from third party applications.
import json
from unittest import TestCase
-from tests.functional import patch, MagicMock
+from tests.functional import patch, MagicMock, call
class SongImportTestHelper(TestCase):
@@ -118,8 +118,11 @@ class SongImportTestHelper(TestCase):
if ccli_number:
self.assertEqual(importer.ccli_number, ccli_number,
'ccli_number for %s should be %s' % (source_file_name, ccli_number))
+ expected_calls = []
for verse_text, verse_tag in add_verse_calls:
self.mocked_add_verse.assert_any_call(verse_text, verse_tag)
+ expected_calls.append(call(verse_text, verse_tag))
+ self.mocked_add_verse.assert_has_calls(expected_calls, any_order=False)
if topics:
self.assertEqual(importer.topics, topics, 'topics for %s should be %s' % (source_file_name, topics))
if comments:
diff --git a/tests/resources/opensongsongs/Beautiful Garden Of Prayer b/tests/resources/opensongsongs/Beautiful Garden Of Prayer
index 33d4943bd..29cd26cc8 100644
--- a/tests/resources/opensongsongs/Beautiful Garden Of Prayer
+++ b/tests/resources/opensongsongs/Beautiful Garden Of Prayer
@@ -16,6 +16,7 @@
;Test breaks and newlines
;A single | on the end of a line adds an extra \n
;Blank lines are ignored, even with a space prefix
+;We also check that the chorus is added after the verses, despite the order in the file
[V1]
There's a garden where Jesus is waiting,
@@ -23,6 +24,14 @@
For it glows with the light of His presence,|
'Tis the beautiful garden of prayer.
+;A double || on a line adds a new slide
+[C]
+ O the beautiful garden, the garden of prayer,
+ O the beautiful garden of prayer.
+ There my Savior awaits, and He opens the gates
+ ||
+ To the beautiful garden of prayer.
+
;A double || on the end of a line adds a new slide
[V2]
There's a garden where Jesus is waiting,
@@ -37,14 +46,6 @@
Just to bow and receive a new blessing,
|
In the beautiful garden of prayer.
-
-;A double || on a line adds a new slide
-[C]
- O the beautiful garden, the garden of prayer,
- O the beautiful garden of prayer.
- There my Savior awaits, and He opens the gates
- ||
- To the beautiful garden of prayer.
DS0
diff --git a/tests/resources/opensongsongs/One, Two, Three, Four, Five b/tests/resources/opensongsongs/One, Two, Three, Four, Five
new file mode 100644
index 000000000..4cd0d894f
--- /dev/null
+++ b/tests/resources/opensongsongs/One, Two, Three, Four, Five
@@ -0,0 +1,37 @@
+
+
+ 12345
+ Traditional
+ Public Domain
+ T
+
+
+
+
+
+
+
+
+;Test [T]ag element - should be turned into [o]ther
+;And lines beginning with numbers
+;And a title that contains only numeric characters
+;That isdiffernt to the filename
+;And most elements are empty
+[T]
+ 1, 2, 3, 4, 5,
+ Once I caught a fish alive.
+ 6, 7, 8, 9, 10,
+ Then I let it go again.
+
+ Why did you let it go?
+ Because it bit my finger so.
+ Which finger did it bite?
+ This little finger on my right.
+
+
+
+
+
+
+
+
\ No newline at end of file
diff --git a/tests/resources/opensongsongs/One, Two, Three, Four, Five.json b/tests/resources/opensongsongs/One, Two, Three, Four, Five.json
new file mode 100644
index 000000000..30fe71c64
--- /dev/null
+++ b/tests/resources/opensongsongs/One, Two, Three, Four, Five.json
@@ -0,0 +1,17 @@
+{
+ "authors": [
+ "Traditional"
+ ],
+ "comments": "",
+ "copyright": "Public Domain ",
+ "title": "12345",
+ "topics": [
+ ],
+ "verse_order_list": ["o1"],
+ "verses": [
+ [
+ "1, 2, 3, 4, 5,\nOnce I caught a fish alive.\n6, 7, 8, 9, 10,\nThen I let it go again.\nWhy did you let it go?\nBecause it bit my finger so.\nWhich finger did it bite?\nThis little finger on my right.",
+ "o1"
+ ]
+ ]
+}
\ No newline at end of file