Tweaks from review

openlp/plugins/songs/lib/opensongimport.py

@@ -26,11 +26,12 @@
 import os
 import re
 
-from songimport import SongImport
+from zipfile import ZipFile
+
 from lxml.etree import Element
 from lxml import objectify
 
-from zipfile import ZipFile
+from openlp.plugins.songs.lib.songimport import SongImport
 
 import logging
 log = logging.getLogger(__name__)
@@ -104,12 +105,12 @@ class OpenSongImport(object):
         set False, the import will not be committed to the database
         (useful for test scripts)
         """
-        ext=os.path.splitext(filename)[1]
+        ext = os.path.splitext(filename)[1]
         if ext.lower() == ".zip":
             log.info('Zipfile found %s', filename)
-            z=ZipFile(filename, u'r')
+            z = ZipFile(filename, u'r')
             for song in z.infolist():
-                parts=os.path.split(song.filename)
+                parts = os.path.split(song.filename)
                 if parts[-1] == u'':
                     #No final part => directory
                     continue
@@ -158,28 +159,29 @@ class OpenSongImport(object):
         # in the absence of any other indication, verses are the default,
         # erm, versetype!
         versetype = u'V'
-        for l in lyrics.split(u'\n'):
+        for thisline in lyrics.split(u'\n'):
             # remove comments
-            semicolon = l.find(u';')
+            semicolon = thisline.find(u';')
             if semicolon >= 0:
-                l = l[:semicolon]
+                thisline = thisline[:semicolon]
-            l = l.strip()
+            thisline = thisline.strip()
-            if len(l) == 0:
+            if len(thisline) == 0:
                 continue
-            # skip inline guitar chords and page and column breaks
+            # skip inthisline guitar chords and page and column breaks
-            if l[0] == u'.' or l.startswith(u'---') or l.startswith(u'-!!'):
+            if thisline[0] == u'.' or thisline.startswith(u'---') \
+                or thisline.startswith(u'-!!'):
                 continue
             
             # verse/chorus/etc. marker
-            if l[0] == u'[':
+            if thisline[0] == u'[':
-                versetype = l[1].upper()
+                versetype = thisline[1].upper()
                 if versetype.isdigit():
                     versenum = versetype
                     versetype = u'V'
-                elif l[2] != u']':
+                elif thisline[2] != u']':
                     # there's a number to go with it - extract that as well
-                    right_bracket = l.find(u']')
+                    right_bracket = thisline.find(u']')
-                    versenum = l[2:right_bracket]
+                    versenum = thisline[2:right_bracket]
                 else:
                     # if there's no number, assume it's no.1
                     versenum = u'1'
@@ -187,13 +189,13 @@ class OpenSongImport(object):
             words = None
 
             # number at start of line.. it's verse number
-            if l[0].isdigit():
+            if thisline[0].isdigit():
-                versenum = l[0]
+                versenum = thisline[0]
-                words = l[1:].strip()
+                words = thisline[1:].strip()
             if words is None and \
                    versenum is not None and \
                    versetype is not None:
-                words = l
+                words = thisline
             if versenum is not None:
                 versetag = u'%s%s'%(versetype,versenum)
                 if not verses.has_key(versetype):
@@ -205,20 +207,20 @@ class OpenSongImport(object):
                     our_verse_order.append(versetag)
             if words:
                 # Tidy text and remove the ____s from extended words
-                words=self.song_import.tidy_text(words)
+                words = self.song_import.tidy_text(words)
-                words=words.replace('_', '')
+                words = words.replace('_', '')
                 verses[versetype][versenum].append(words)
         # done parsing
         versetypes = verses.keys()
         versetypes.sort()
         versetags = {}
         verse_renames = {}
-        for v in versetypes:
+        for versetype in versetypes:
-            versenums = verses[v].keys()
+            versenums = verses[versetype].keys()
             versenums.sort()
-            for n in versenums:
+            for num in versenums:
-                versetag = u'%s%s' %(v,n)
+                versetag = u'%s%s' %(versetype,num)
-                lines = u'\n'.join(verses[v][n])
+                lines = u'\n'.join(verses[versetype][num])
                 self.song_import.verses.append([versetag, lines])
                 versetags[versetag] = 1 # keep track of what we have for error checking later
         # now figure out the presentation order

openlp/plugins/songs/lib/test/test_opensongimport.py

@@ -32,17 +32,16 @@ def test():
     manager = Manager(u'songs', init_schema)
     o = OpenSongImport(manager)
     o.do_import(u'test.opensong', commit=False)
-    o.finish()
     o.song_import.print_song()
     assert o.song_import.copyright == u'2010 Martin Thompson'
     assert o.song_import.authors == [u'MartiÑ Thómpson']
     assert o.song_import.title == u'Martins Test'
     assert o.song_import.alternate_title == u''
     assert o.song_import.song_number == u'1'
-    assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.song_import.verses 
     assert [u'C1', u'Chorus 1'] in o.song_import.verses 
     assert [u'C2', u'Chorus 2'] in o.song_import.verses 
     assert not [u'C3', u'Chorus 3'] in o.song_import.verses 
+    assert [u'B1', u'Bridge 1\nBridge 1 line 2'] in o.song_import.verses 
     assert [u'V1', u'v1 Line 1\nV1 Line 2'] in o.song_import.verses 
     assert [u'V2', u'v2 Line 1\nV2 Line 2'] in o.song_import.verses
     assert o.song_import.verse_order_list == [u'V1', u'C1', u'V2', u'C2', u'V3', u'B1', u'V1']

openlp/plugins/songs/lib/xml.py

@@ -77,7 +77,8 @@ class SongXMLBuilder(object):
             The actual text of the verse to be stored.
         """
         # log.debug(u'add_verse_to_lyrics %s, %s\n%s' % (type, number, content))
-        verse = etree.Element(u'verse', type = unicode(type), label = unicode(number))
+        verse = etree.Element(u'verse', type = unicode(type),
+            label = unicode(number))
         verse.text = etree.CDATA(content)
         self.lyrics.append(verse)