- improved the song importers by allowing to provide feedback to the user.

- Do not abort a song import, always try to continue and log which songs could not be imported.
- Clean ups (close files, deduplication, ...)

bzr-revno: 1483

openlp/core/ui/wizard.py

@@ -95,6 +95,10 @@ class OpenLPWizard(QtGui.QWizard):
         self.customSignals()
         QtCore.QObject.connect(self, QtCore.SIGNAL(u'currentIdChanged(int)'),
             self.onCurrentIdChanged)
+        QtCore.QObject.connect(self.errorCopyToButton,
+            QtCore.SIGNAL(u'clicked()'), self.onErrorCopyToButtonClicked)
+        QtCore.QObject.connect(self.errorSaveToButton,
+            QtCore.SIGNAL(u'clicked()'), self.onErrorSaveToButtonClicked)
 
     def setupUi(self, image):
         """
@@ -129,10 +133,36 @@ class OpenLPWizard(QtGui.QWizard):
         self.progressLayout.setObjectName(u'progressLayout')
         self.progressLabel = QtGui.QLabel(self.progressPage)
         self.progressLabel.setObjectName(u'progressLabel')
+        self.progressLabel.setWordWrap(True)
         self.progressLayout.addWidget(self.progressLabel)
         self.progressBar = QtGui.QProgressBar(self.progressPage)
         self.progressBar.setObjectName(u'progressBar')
         self.progressLayout.addWidget(self.progressBar)
+        # Add a QTextEdit and a copy to file and copy to clipboard button to be
+        # able to provide feedback to the user. Hidden by default.
+        self.errorReportTextEdit = QtGui.QTextEdit(self.progressPage)
+        self.errorReportTextEdit.setObjectName(u'progresserrorReportTextEdit')
+        self.errorReportTextEdit.setHidden(True)
+        self.errorReportTextEdit.setReadOnly(True)
+        self.progressLayout.addWidget(self.errorReportTextEdit)
+        self.errorButtonLayout = QtGui.QHBoxLayout()
+        self.errorButtonLayout.setObjectName(u'errorButtonLayout')
+        spacer = QtGui.QSpacerItem(40, 20,
+            QtGui.QSizePolicy.Expanding, QtGui.QSizePolicy.Minimum)
+        self.errorButtonLayout.addItem(spacer)
+        self.errorCopyToButton = QtGui.QPushButton(self.progressPage)
+        self.errorCopyToButton.setObjectName(u'errorCopyToButton')
+        self.errorCopyToButton.setHidden(True)
+        self.errorCopyToButton.setIcon(
+            build_icon(u':/system/system_edit_copy.png'))
+        self.errorButtonLayout.addWidget(self.errorCopyToButton)
+        self.errorSaveToButton = QtGui.QPushButton(self.progressPage)
+        self.errorSaveToButton.setObjectName(u'errorSaveToButton')
+        self.errorSaveToButton.setHidden(True)
+        self.errorSaveToButton.setIcon(
+            build_icon(u':/general/general_save.png'))
+        self.errorButtonLayout.addWidget(self.errorSaveToButton)
+        self.progressLayout.addLayout(self.errorButtonLayout)
         self.addPage(self.progressPage)
 
     def exec_(self):
@@ -160,6 +190,18 @@ class OpenLPWizard(QtGui.QWizard):
             self.performWizard()
             self.postWizard()
 
+    def onErrorCopyToButtonClicked(self):
+        """
+        Called when the ``onErrorCopyToButtonClicked`` has been clicked.
+        """
+        pass
+
+    def onErrorSaveToButtonClicked(self):
+        """
+        Called when the ``onErrorSaveToButtonClicked`` has been clicked.
+        """
+        pass
+
     def incrementProgressBar(self, status_text, increment=1):
         """
         Update the wizard progress page.
@@ -220,3 +262,4 @@ class OpenLPWizard(QtGui.QWizard):
             editbox.setText(filename)
             SettingsManager.set_last_dir(self.plugin.settingsSection,
                 filename, 1)
+

openlp/plugins/songs/forms/songimportform.py

@@ -26,6 +26,7 @@
 """
 The song import functions for OpenLP.
 """
+import codecs
 import logging
 import os
 
@@ -55,6 +56,7 @@ class SongImportForm(OpenLPWizard):
         ``plugin``
             The songs plugin.
         """
+        self.clipboard = plugin.formparent.clipboard
         OpenLPWizard.__init__(self, parent, plugin, u'songImportWizard',
             u':/wizards/wizard_importsong.bmp')
 
@@ -330,6 +332,10 @@ class SongImportForm(OpenLPWizard):
                 'Please wait while your songs are imported.'))
         self.progressLabel.setText(WizardStrings.Ready)
         self.progressBar.setFormat(WizardStrings.PercentSymbolFormat)
+        self.errorCopyToButton.setText(translate('SongsPlugin.ImportWizardForm',
+            'Copy'))
+        self.errorSaveToButton.setText(translate('SongsPlugin.ImportWizardForm',
+            'Save to File'))
         # Align all QFormLayouts towards each other.
         width = max(self.formatLabel.minimumSizeHint().width(),
             self.openLP2FilenameLabel.minimumSizeHint().width())
@@ -459,10 +465,7 @@ class SongImportForm(OpenLPWizard):
         """
         Return a list of file from the listbox
         """
-        files = []
+        return [unicode(listbox.item(i).text()) for i in range(listbox.count())]
-        for row in range(0, listbox.count()):
-            files.append(unicode(listbox.item(row).text()))
-        return files
 
     def removeSelectedItems(self, listbox):
         """
@@ -659,6 +662,10 @@ class SongImportForm(OpenLPWizard):
         self.songShowPlusFileListWidget.clear()
         self.foilPresenterFileListWidget.clear()
         #self.csvFilenameEdit.setText(u'')
+        self.errorReportTextEdit.clear()
+        self.errorReportTextEdit.setHidden(True)
+        self.errorCopyToButton.setHidden(True)
+        self.errorSaveToButton.setHidden(True)
 
     def preWizard(self):
         """
@@ -743,12 +750,30 @@ class SongImportForm(OpenLPWizard):
             importer = self.plugin.importSongs(SongFormat.FoilPresenter,
                 filenames=self.getListOfFiles(self.foilPresenterFileListWidget)
             )
-        if importer.do_import():
+        importer.do_import()
-            self.progressLabel.setText(WizardStrings.FinishedImport)
+        if importer.error_log:
+            self.progressLabel.setText(translate(
+                'SongsPlugin.SongImportForm', 'Your song import failed.'))
         else:
-            self.progressLabel.setText(
+            self.progressLabel.setText(WizardStrings.FinishedImport)
-                translate('SongsPlugin.SongImportForm',
+
-                'Your song import failed.'))
+    def onErrorCopyToButtonClicked(self):
+        """
+        Copy the error report to the clipboard.
+        """
+        self.clipboard.setText(self.errorReportTextEdit.toPlainText())
+
+    def onErrorSaveToButtonClicked(self):
+        """
+        Save the error report to a file.
+        """
+        filename = QtGui.QFileDialog.getSaveFileName(self,
+            SettingsManager.get_last_dir(self.plugin.settingsSection, 1))
+        if not filename:
+            return
+        file = codecs.open(filename, u'w', u'utf-8')
+        file.write(self.errorReportTextEdit.toPlainText())
+        file.close()
 
     def addFileSelectItem(self, prefix, obj_prefix=None, can_disable=False,
         single_select=False):

openlp/plugins/songs/lib/cclifileimport.py

@@ -59,16 +59,10 @@ class CCLIFileImport(SongImport):
         Import either a ``.usr`` or a ``.txt`` SongSelect file.
         """
         log.debug(u'Starting CCLI File Import')
-        song_total = len(self.import_source)
+        self.import_wizard.progressBar.setMaximum(len(self.import_source))
-        self.import_wizard.progressBar.setMaximum(song_total)
-        song_count = 1
         for filename in self.import_source:
-            self.import_wizard.incrementProgressBar(unicode(translate(
-                'SongsPlugin.CCLIFileImport', 'Importing song %d of %d')) %
-                (song_count, song_total))
             filename = unicode(filename)
             log.debug(u'Importing CCLI File: %s', filename)
-            self.set_defaults()
             lines = []
             if os.path.isfile(filename):
                 detect_file = open(filename, u'r')
@@ -81,19 +75,23 @@ class CCLIFileImport(SongImport):
                 detect_file.close()
                 infile = codecs.open(filename, u'r', details['encoding'])
                 lines = infile.readlines()
+                infile.close()
                 ext = os.path.splitext(filename)[1]
                 if ext.lower() == u'.usr':
                     log.info(u'SongSelect .usr format file found: %s', filename)
-                    self.do_import_usr_file(lines)
+                    if not self.do_import_usr_file(lines):
+                        self.log_error(filename)
                 elif ext.lower() == u'.txt':
                     log.info(u'SongSelect .txt format file found: %s', filename)
-                    self.do_import_txt_file(lines)
+                    if not self.do_import_txt_file(lines):
+                        self.log_error(filename)
                 else:
+                    self.log_error(filename,
+                        translate('SongsPlugin.CCLIFileImport',
+                        'The file does not have a valid extension.'))
                     log.info(u'Extension %s is not valid', filename)
-                song_count += 1
             if self.stop_import_flag:
-                return False
+                return
-        return True
 
     def do_import_usr_file(self, textList):
         """
@@ -218,7 +216,7 @@ class CCLIFileImport(SongImport):
             else:
                 self.add_author(author)
         self.topics = [topic.strip() for topic in song_topics.split(u'/t')]
-        self.finish()
+        return self.finish()
 
     def do_import_txt_file(self, textList):
         """
@@ -334,6 +332,5 @@ class CCLIFileImport(SongImport):
         if len(author_list) < 2:
             author_list = song_author.split(u'|')
         # Clean spaces before and after author names.
-        for author_name in author_list:
+        [self.add_author(author_name.strip()) for author_name in author_list]
-            self.add_author(author_name.strip())
+        return self.finish()
-        self.finish()

openlp/plugins/songs/lib/easislidesimport.py

@@ -26,11 +26,13 @@
 
 import logging
 import os
-from lxml import etree, objectify
 import re
 
+from lxml import etree, objectify
+
 from openlp.core.lib import translate
 from openlp.core.ui.wizard import WizardStrings
+from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
 
 log = logging.getLogger(__name__)
@@ -56,26 +58,16 @@ class EasiSlidesImport(SongImport):
         multiple opensong files. If `self.commit` is set False, the
         import will not be committed to the database (useful for test scripts).
         """
-        self.import_wizard.progressBar.setMaximum(1)
         log.info(u'Importing EasiSlides XML file %s', self.import_source)
         parser = etree.XMLParser(remove_blank_text=True)
         file = etree.parse(self.import_source, parser)
         xml = unicode(etree.tostring(file))
         song_xml = objectify.fromstring(xml)
-        self.import_wizard.incrementProgressBar(
-            WizardStrings.ImportingType % os.path.split(self.import_source)[-1])
         self.import_wizard.progressBar.setMaximum(len(song_xml.Item))
         for song in song_xml.Item:
-            self.import_wizard.incrementProgressBar(
+            if self.stop_import_flag:
-                unicode(translate('SongsPlugin.ImportWizardForm',
+                return
-                    u'Importing %s, song %s...')) %
+            self._parse_song(song)
-                    (os.path.split(self.import_source)[-1], song.Title1))
-            success = self._parse_song(song)
-            if not success or self.stop_import_flag:
-                return False
-            elif self.commit:
-                self.finish()
-        return True
 
     def _parse_song(self, song):
         self._success = True
@@ -90,7 +82,11 @@ class EasiSlidesImport(SongImport):
         self._add_copyright(song.LicenceAdmin2)
         self._add_unicode_attribute(u'song_book_name', song.BookReference)
         self._parse_and_add_lyrics(song)
-        return self._success
+        if self._success:
+            if not self.finish():
+                self.log_error(song.Title1 if song.Title1 else u'')
+        else:
+            self.set_defaults()
 
     def _add_unicode_attribute(self, self_attribute, import_attribute,
         mandatory=False):
@@ -122,10 +118,8 @@ class EasiSlidesImport(SongImport):
     def _add_authors(self, song):
         try:
             authors = unicode(song.Writer).split(u',')
-            for author in authors:
+            self.authors = \
-                author = author.strip()
+                [author.strip() for author in authors if author.strip()]
-                if len(author):
-                    self.authors.append(author)
         except UnicodeDecodeError:
             log.exception(u'Unicode decode error while decoding Writer')
             self._success = False
@@ -188,12 +182,13 @@ class EasiSlidesImport(SongImport):
         # if the regions are inside verses
         regionsInVerses = (regions and regionlines[regionlines.keys()[0]] > 1)
         MarkTypes = {
-            u'CHORUS': u'C',
+            u'CHORUS': VerseType.Tags[VerseType.Chorus],
-            u'VERSE': u'V',
+            u'VERSE': VerseType.Tags[VerseType.Verse],
-            u'INTRO': u'I',
+            u'INTRO': VerseType.Tags[VerseType.Intro],
-            u'ENDING': u'E',
+            u'ENDING': VerseType.Tags[VerseType.Ending],
-            u'BRIDGE': u'B',
+            u'BRIDGE': VerseType.Tags[VerseType.Bridge],
-            u'PRECHORUS': u'P'}
+            u'PRECHORUS': VerseType.Tags[VerseType.PreChorus]
+        }
         verses = {}
         # list as [region, versetype, versenum, instance]
         our_verse_order = []

openlp/plugins/songs/lib/ewimport.py

@@ -33,6 +33,7 @@ import struct
 
 from openlp.core.lib import translate
 from openlp.core.ui.wizard import WizardStrings
+from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib import retrieve_windows_encoding
 from songimport import SongImport
 
@@ -142,12 +143,12 @@ class EasyWorshipSongImport(SongImport):
         # Open the DB and MB files if they exist
         import_source_mb = self.import_source.replace('.DB', '.MB')
         if not os.path.isfile(self.import_source):
-            return False
+            return
         if not os.path.isfile(import_source_mb):
-            return False
+            return
         db_size = os.path.getsize(self.import_source)
         if db_size < 0x800:
-            return False
+            return
         db_file = open(self.import_source, 'rb')
         self.memo_file = open(import_source_mb, 'rb')
         # Don't accept files that are clearly not paradox files
@@ -156,7 +157,7 @@ class EasyWorshipSongImport(SongImport):
         if header_size != 0x800 or block_size < 1 or block_size > 4:
             db_file.close()
             self.memo_file.close()
-            return False
+            return
         # Take a stab at how text is encoded
         self.encoding = u'cp1252'
         db_file.seek(106)
@@ -183,7 +184,7 @@ class EasyWorshipSongImport(SongImport):
             self.encoding = u'cp874'
         self.encoding = retrieve_windows_encoding(self.encoding)
         if not self.encoding:
-            return False
+            return
         # There does not appear to be a _reliable_ way of getting the number
         # of songs/records, so let's use file blocks for measuring progress.
         total_blocks = (db_size - header_size) / (block_size * 1024)
@@ -203,8 +204,8 @@ class EasyWorshipSongImport(SongImport):
                 field_size))
         self.set_record_struct(field_descs)
         # Pick out the field description indexes we will need
-        success = True
         try:
+            success = True
             fi_title = self.find_field(u'Title')
             fi_author = self.find_field(u'Author')
             fi_copy = self.find_field(u'Copyright')
@@ -223,31 +224,25 @@ class EasyWorshipSongImport(SongImport):
             # Loop through each record within the current block
             for i in range(rec_count):
                 if self.stop_import_flag:
-                    success = False
                     break
                 raw_record = db_file.read(record_size)
                 self.fields = self.record_struct.unpack(raw_record)
                 self.set_defaults()
-                # Get title and update progress bar message
+                self.title = self.get_field(fi_title)
-                title = self.get_field(fi_title)
+                # Get remaining fields.
-                if title:
-                    self.import_wizard.incrementProgressBar(
-                        WizardStrings.ImportingType % title, 0)
-                    self.title = title
-                # Get remaining fields
                 copy = self.get_field(fi_copy)
                 admin = self.get_field(fi_admin)
                 ccli = self.get_field(fi_ccli)
                 authors = self.get_field(fi_author)
                 words = self.get_field(fi_words)
-                # Set the SongImport object members
+                # Set the SongImport object members.
                 if copy:
                     self.copyright = copy
                 if admin:
                     if copy:
                         self.copyright += u', '
                     self.copyright += \
-                        unicode(translate('SongsPlugin.ImportWizardForm',
+                        unicode(translate('SongsPlugin.EasyWorshipSongImport',
                             'Administered by %s')) % admin
                 if ccli:
                     self.ccli_number = ccli
@@ -264,19 +259,17 @@ class EasyWorshipSongImport(SongImport):
                     # Format the lyrics
                     words = strip_rtf(words, self.encoding)
                     for verse in words.split(u'\n\n'):
-                        self.add_verse(verse.strip(), u'V')
+                        self.add_verse(
+                            verse.strip(), VerseType.Tags[VerseType.Verse])
                 if self.stop_import_flag:
-                    success = False
                     break
-                self.finish()
+                if not self.finish():
-            if not self.stop_import_flag:
+                    self.log_error(self.import_source)
-                self.import_wizard.incrementProgressBar(u'')
         db_file.close()
         self.memo_file.close()
-        return success
 
     def find_field(self, field_name):
-        return [i for i, x in enumerate(self.field_descs) \
+        return [i for i, x in enumerate(self.field_descs)
             if x.name == field_name][0]
 
     def set_record_struct(self, field_descs):

openlp/plugins/songs/lib/foilpresenterimport.py

@@ -97,6 +97,7 @@ from openlp.core.ui.wizard import WizardStrings
 from openlp.plugins.songs.lib import clean_song, VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
 from openlp.plugins.songs.lib.db import Author, Book, Song, Topic
+from openlp.plugins.songs.lib.ui import SongStrings
 from openlp.plugins.songs.lib.xml import SongXML
 
 log = logging.getLogger(__name__)
@@ -121,17 +122,16 @@ class FoilPresenterImport(SongImport):
         parser = etree.XMLParser(remove_blank_text=True)
         for file_path in self.import_source:
             if self.stop_import_flag:
-                return False
+                return
             self.import_wizard.incrementProgressBar(
                 WizardStrings.ImportingType % os.path.basename(file_path))
             try:
                 parsed_file = etree.parse(file_path, parser)
                 xml = unicode(etree.tostring(parsed_file))
-                if self.FoilPresenter.xml_to_song(xml) is None:
+                self.FoilPresenter.xml_to_song(xml)
-                    log.debug(u'File could not be imported: %s' % file_path)
             except etree.XMLSyntaxError:
+                self.log_error(file_path, SongStrings.XMLSyntaxError)
                 log.exception(u'XML syntax error in file %s' % file_path)
-        return True
 
 
 class FoilPresenter(object):
@@ -211,7 +211,7 @@ class FoilPresenter(object):
         """
         # No xml get out of here.
         if not xml:
-            return None
+            return
         if xml[:5] == u'<?xml':
             xml = xml[38:]
         song = Song()
@@ -235,7 +235,6 @@ class FoilPresenter(object):
         self._process_topics(foilpresenterfolie, song)
         clean_song(self.manager, song)
         self.manager.save_object(song)
-        return song.id
 
     def _child(self, element):
         """
@@ -305,9 +304,8 @@ class FoilPresenter(object):
             for marker in markers:
                 copyright = re.compile(marker).sub(u'<marker>', copyright, re.U)
             copyright = re.compile(u'(?<=<marker>) *:').sub(u'', copyright)
-            i = 0
             x = 0
-            while i != 1:
+            while True:
                 if copyright.find(u'<marker>') != -1:
                     temp = copyright.partition(u'<marker>')
                     if temp[0].strip() and x > 0:
@@ -316,9 +314,9 @@ class FoilPresenter(object):
                     x += 1
                 elif x > 0:
                     strings.append(copyright)
-                    i = 1
+                    break
                 else:
-                    i = 1
+                    break
             author_temp = []
             for author in strings:
                 temp = re.split(u',(?=\D{2})|(?<=\D),|\/(?=\D{3,})|(?<=\D);',
@@ -414,8 +412,15 @@ class FoilPresenter(object):
         temp_verse_order_backup = []
         temp_sortnr_backup = 1
         temp_sortnr_liste = []
-        versenumber = {u'V': 1, u'C': 1, u'B': 1, u'E': 1, u'O': 1, u'I': 1,
+        versenumber = {
-            u'P': 1}
+            VerseType.Tags[VerseType.Verse]: 1,
+            VerseType.Tags[VerseType.Chorus]: 1, 
+            VerseType.Tags[VerseType.Bridge]: 1,
+            VerseType.Tags[VerseType.Ending]: 1,
+            VerseType.Tags[VerseType.Other]: 1,
+            VerseType.Tags[VerseType.Intro]: 1,
+            VerseType.Tags[VerseType.PreChorus]: 1
+        }
         for strophe in foilpresenterfolie.strophen.strophe:
             text = self._child(strophe.text_)
             verse_name = self._child(strophe.key)
@@ -434,25 +439,25 @@ class FoilPresenter(object):
             temp_verse_name = re.compile(u'[0-9].*').sub(u'', verse_name)
             temp_verse_name = temp_verse_name[:3].lower()
             if temp_verse_name == u'ref':
-                verse_type = u'C'
+                verse_type = VerseType.Tags[VerseType.Chorus]
             elif temp_verse_name == u'r':
-                verse_type = u'C'
+                verse_type = VerseType.Tags[VerseType.Chorus]
             elif temp_verse_name == u'':
-                verse_type = u'V'
+                verse_type = VerseType.Tags[VerseType.Verse]
             elif temp_verse_name == u'v':
-                verse_type = u'V'
+                verse_type = VerseType.Tags[VerseType.Verse]
             elif temp_verse_name == u'bri':
-                verse_type = u'B'
+                verse_type = VerseType.Tags[VerseType.Bridge]
             elif temp_verse_name == u'cod':
-                verse_type = u'E'
+                verse_type = VerseType.Tags[VerseType.Ending]
             elif temp_verse_name == u'sch':
-                verse_type = u'E'
+                verse_type = VerseType.Tags[VerseType.Ending]
             elif temp_verse_name == u'pre':
-                verse_type = u'P'
+                verse_type = VerseType.Tags[VerseType.PreChorus]
             elif temp_verse_name == u'int':
-                verse_type = u'I'
+                verse_type = VerseType.Tags[VerseType.Intro]
             else:
-                verse_type = u'O'
+                verse_type = VerseType.Tags[VerseType.Other]
             verse_number = re.compile(u'[a-zA-Z.+-_ ]*').sub(u'', verse_name)
             # Foilpresenter allows e. g. "C", but we need "C1".
             if not verse_number:
@@ -466,8 +471,8 @@ class FoilPresenter(object):
                         verse_number = unicode(int(verse_number) + 1)
             verse_type_index = VerseType.from_tag(verse_type[0])
             verse_type = VerseType.Names[verse_type_index]
-            temp_verse_order[verse_sortnr] = (u''.join((verse_type[0],
+            temp_verse_order[verse_sortnr] = u''.join((verse_type[0],
-                verse_number)))
+                verse_number))
             temp_verse_order_backup.append(u''.join((verse_type[0],
                 verse_number)))
             sxml.add_verse_to_lyrics(verse_type, verse_number, text)

openlp/plugins/songs/lib/olp1import.py

@@ -32,7 +32,7 @@ import logging
 from chardet.universaldetector import UniversalDetector
 import sqlite
 
-from openlp.core.ui.wizard import WizardStrings
+from openlp.core.lib import translate
 from openlp.plugins.songs.lib import retrieve_windows_encoding
 from songimport import SongImport
 
@@ -61,10 +61,15 @@ class OpenLP1SongImport(SongImport):
         """
         Run the import for an openlp.org 1.x song database.
         """
-        # Connect to the database
+        if not self.import_source.endswith(u'.olp'):
+            self.log_error(self.import_source,
+                translate('SongsPlugin.OpenLP1SongImport',
+                'Not a valid openlp.org 1.x song database.'))
+            return
         encoding = self.get_encoding()
         if not encoding:
-            return False
+            return
+        # Connect to the database
         connection = sqlite.connect(self.import_source, mode=0444,
             encoding=(encoding, 'replace'))
         cursor = connection.cursor()
@@ -72,12 +77,6 @@ class OpenLP1SongImport(SongImport):
         cursor.execute(u'SELECT name FROM sqlite_master '
             u'WHERE type = \'table\' AND name = \'tracks\'')
         new_db = len(cursor.fetchall()) > 0
-        # Count the number of records we need to import, for the progress bar
-        cursor.execute(u'-- types int')
-        cursor.execute(u'SELECT COUNT(songid) FROM songs')
-        count = cursor.fetchone()[0]
-        success = True
-        self.import_wizard.progressBar.setMaximum(count)
         # "cache" our list of authors
         cursor.execute(u'-- types int, unicode')
         cursor.execute(u'SELECT authorid, authorname FROM authors')
@@ -92,37 +91,29 @@ class OpenLP1SongImport(SongImport):
         cursor.execute(u'SELECT songid, songtitle, lyrics || \'\' AS lyrics, '
             u'copyrightinfo FROM songs')
         songs = cursor.fetchall()
+        self.import_wizard.progressBar.setMaximum(len(songs))
         for song in songs:
             self.set_defaults()
             if self.stop_import_flag:
-                success = False
                 break
             song_id = song[0]
-            title = song[1]
+            self.title = song[1]
             lyrics = song[2].replace(u'\r\n', u'\n')
-            copyright = song[3]
+            self.add_copyright(song[3])
-            self.import_wizard.incrementProgressBar(
-                WizardStrings.ImportingType % title)
-            self.title = title
             verses = lyrics.split(u'\n\n')
-            for verse in verses:
+            [self.add_verse(verse.strip()) for verse in verses if verse.strip()]
-                if verse.strip() != u'':
-                    self.add_verse(verse.strip())
-            self.add_copyright(copyright)
             cursor.execute(u'-- types int')
             cursor.execute(u'SELECT authorid FROM songauthors '
                 u'WHERE songid = %s' % song_id)
             author_ids = cursor.fetchall()
             for author_id in author_ids:
                 if self.stop_import_flag:
-                    success = False
                     break
                 for author in authors:
                     if author[0] == author_id[0]:
                         self.parse_author(author[1])
                         break
             if self.stop_import_flag:
-                success = False
                 break
             if new_db:
                 cursor.execute(u'-- types int')
@@ -131,17 +122,15 @@ class OpenLP1SongImport(SongImport):
                 track_ids = cursor.fetchall()
                 for track_id in track_ids:
                     if self.stop_import_flag:
-                        success = False
                         break
                     for track in tracks:
                         if track[0] == track_id[0]:
                             self.add_media_file(track[1])
                             break
             if self.stop_import_flag:
-                success = False
                 break
-            self.finish()
+            if not self.finish():
-        return success
+                self.log_error(self.import_source)
 
     def get_encoding(self):
         """

openlp/plugins/songs/lib/olpimport.py

@@ -36,6 +36,7 @@ from sqlalchemy.orm.exc import UnmappedClassError
 
 from openlp.core.lib import translate
 from openlp.core.lib.db import BaseModel
+from openlp.core.ui.wizard import WizardStrings
 from openlp.plugins.songs.lib import clean_song
 from openlp.plugins.songs.lib.db import Author, Book, Song, Topic #, MediaFile
 from songimport import SongImport
@@ -93,13 +94,18 @@ class OpenLPSongImport(SongImport):
             The database providing the data to import.
         """
         SongImport.__init__(self, manager, **kwargs)
-        self.import_source = u'sqlite:///%s' % self.import_source
         self.source_session = None
 
     def do_import(self):
         """
         Run the import for an OpenLP version 2 song database.
         """
+        if not self.import_source.endswith(u'.sqlite'):
+            self.log_error(self.import_source,
+                translate('SongsPlugin.OpenLPSongImport',
+                'Not a valid OpenLP 2.0 song database.'))
+            return
+        self.import_source = u'sqlite:///%s' % self.import_source
         engine = create_engine(self.import_source)
         source_meta = MetaData()
         source_meta.reflect(engine)
@@ -150,15 +156,9 @@ class OpenLPSongImport(SongImport):
             mapper(OldTopic, source_topics_table)
 
         source_songs = self.source_session.query(OldSong).all()
-        song_total = len(source_songs)
         if self.import_wizard:
-            self.import_wizard.progressBar.setMaximum(song_total)
+            self.import_wizard.progressBar.setMaximum(len(source_songs))
-        song_count = 1
         for song in source_songs:
-            if self.import_wizard:
-                self.import_wizard.incrementProgressBar(
-                    unicode(translate('SongsPlugin.OpenLPSongImport',
-                    'Importing song %d of %d.')) % (song_count, song_total))
             new_song = Song()
             new_song.title = song.title
             if has_media_files and hasattr(song, 'alternate_title'):
@@ -213,8 +213,9 @@ class OpenLPSongImport(SongImport):
 #                                file_name=media_file.file_name))
             clean_song(self.manager, new_song)
             self.manager.save_object(new_song)
-            song_count += 1
+            if self.import_wizard:
+                self.import_wizard.incrementProgressBar(
+                    WizardStrings.ImportingType % new_song.title)
             if self.stop_import_flag:
-                return False
+                break
         engine.dispose()
-        return True

openlp/plugins/songs/lib/oooimport.py

@@ -56,13 +56,11 @@ class OooImport(SongImport):
         self.process_started = False
 
     def do_import(self):
-        self.stop_import_flag = False
-        self.import_wizard.progressBar.setMaximum(0)
         self.start_ooo()
+        self.import_wizard.progressBar.setMaximum(len(self.import_source))
         for filename in self.import_source:
             if self.stop_import_flag:
-                self.import_wizard.incrementProgressBar(u'Import cancelled', 0)
+                break
-                return
             filename = unicode(filename)
             if os.path.isfile(filename):
                 self.open_ooo_file(filename)
@@ -70,9 +68,6 @@ class OooImport(SongImport):
                     self.process_ooo_document()
                     self.close_ooo_file()
         self.close_ooo()
-        self.import_wizard.progressBar.setMaximum(1)
-        self.import_wizard.incrementProgressBar(u'', 1)
-        return True
 
     def process_ooo_document(self):
         """

openlp/plugins/songs/lib/openlyricsimport.py

@@ -35,6 +35,7 @@ from lxml import etree
 
 from openlp.core.ui.wizard import WizardStrings
 from openlp.plugins.songs.lib.songimport import SongImport
+from openlp.plugins.songs.lib.ui import SongStrings
 from openlp.plugins.songs.lib import OpenLyrics
 
 log = logging.getLogger(__name__)
@@ -59,7 +60,7 @@ class OpenLyricsImport(SongImport):
         parser = etree.XMLParser(remove_blank_text=True)
         for file_path in self.import_source:
             if self.stop_import_flag:
-                return False
+                return
             self.import_wizard.incrementProgressBar(
                 WizardStrings.ImportingType % os.path.basename(file_path))
             try:
@@ -67,8 +68,7 @@ class OpenLyricsImport(SongImport):
                 # special characters in the path (see lp:757673 and lp:744337).
                 parsed_file = etree.parse(open(file_path, u'r'), parser)
                 xml = unicode(etree.tostring(parsed_file))
-                if self.openLyrics.xml_to_song(xml) is None:
+                self.openLyrics.xml_to_song(xml)
-                    log.debug(u'File could not be imported: %s' % file_path)
             except etree.XMLSyntaxError:
                 log.exception(u'XML syntax error in file %s' % file_path)
-        return True
+                self.log_error(file_path, SongStrings.XMLSyntaxError)

openlp/plugins/songs/lib/opensongimport.py

@@ -26,13 +26,15 @@
 
 import logging
 import os
+import re
 from zipfile import ZipFile
+
 from lxml import objectify
 from lxml.etree import Error, LxmlError
-import re
 
-from openlp.core.ui.wizard import WizardStrings
+from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
+from openlp.plugins.songs.lib.ui import SongStrings
 
 log = logging.getLogger(__name__)
 
@@ -105,77 +107,62 @@ class OpenSongImport(SongImport):
         Initialise the class.
         """
         SongImport.__init__(self, manager, **kwargs)
-        self.commit = True
 
     def do_import(self):
         """
         Import either each of the files in self.import_source - each element of
         which can be either a single opensong file, or a zipfile containing
-        multiple opensong files. If `self.commit` is set False, the
+        multiple opensong files.
-        import will not be committed to the database (useful for test scripts).
         """
-        success = True
         numfiles = 0
         for filename in self.import_source:
             ext = os.path.splitext(filename)[1]
             if ext.lower() == u'.zip':
                 z = ZipFile(filename, u'r')
                 numfiles += len(z.infolist())
+                z.close()
             else:
                 numfiles += 1
         log.debug(u'Total number of files: %d', numfiles)
         self.import_wizard.progressBar.setMaximum(numfiles)
         for filename in self.import_source:
             if self.stop_import_flag:
-                success = False
+                return
-                break
             ext = os.path.splitext(filename)[1]
             if ext.lower() == u'.zip':
                 log.debug(u'Zipfile found %s', filename)
                 z = ZipFile(filename, u'r')
                 for song in z.infolist():
                     if self.stop_import_flag:
-                        success = False
+                        z.close()
-                        break
+                        return
                     parts = os.path.split(song.filename)
                     if parts[-1] == u'':
                         # No final part => directory
                         continue
                     log.info(u'Zip importing %s', parts[-1])
-                    self.import_wizard.incrementProgressBar(
+                    song_file = z.open(song)
-                        WizardStrings.ImportingType % parts[-1])
+                    self.do_import_file(song_file)
-                    songfile = z.open(song)
+                    song_file.close()
-                    if self.do_import_file(songfile) and self.commit and \
+                z.close()
-                        not self.stop_import_flag:
-                        self.finish()
-                    else:
-                        success = False
-                        break
             else:
                 # not a zipfile
                 log.info(u'Direct import %s', filename)
-                self.import_wizard.incrementProgressBar(
-                    WizardStrings.ImportingType % os.path.split(filename)[-1])
                 song_file = open(filename)
-                if self.do_import_file(song_file) and self.commit and \
+                self.do_import_file(song_file)
-                    not self.stop_import_flag:
+                song_file.close()
-                    self.finish()
-                else:
-                    success = False
-                    break
-        return success
 
     def do_import_file(self, file):
         """
-        Process the OpenSong file - pass in a file-like object,
+        Process the OpenSong file - pass in a file-like object, not a file path.
-        not a filename
         """
         self.set_defaults()
         try:
             tree = objectify.parse(file)
         except (Error, LxmlError):
+            self.log_error(file.name, SongStrings.XMLSyntaxError)
             log.exception(u'Error parsing XML')
-            return False
+            return
         root = tree.getroot()
         fields = dir(root)
         decode = {
@@ -193,9 +180,6 @@ class OpenSongImport(SongImport):
                     setattr(self, fn_or_string, ustring)
                 else:
                     fn_or_string(ustring)
-        if not len(self.title):
-            # to prevent creation of empty songs from wrong files
-            return False
         if u'theme' in fields and unicode(root.theme) not in self.topics:
             self.topics.append(unicode(root.theme))
         if u'alttheme' in fields and unicode(root.alttheme) not in self.topics:
@@ -205,7 +189,7 @@ class OpenSongImport(SongImport):
         # keep track of verses appearance order
         our_verse_order = []
         # default verse
-        verse_tag = u'v'
+        verse_tag = VerseType.Tags[VerseType.Verse]
         verse_num = u'1'
         # for the case where song has several sections with same marker
         inst = 1
@@ -269,7 +253,7 @@ class OpenSongImport(SongImport):
             lines = u'\n'.join(verses[verse_tag][verse_num][inst])
             self.add_verse(lines, verse_def)
         # figure out the presentation order, if present
-        if u'presentation' in fields and root.presentation != u'':
+        if u'presentation' in fields and root.presentation:
             order = unicode(root.presentation)
             # We make all the tags in the lyrics lower case, so match that here
             # and then split into a list on the whitespace
@@ -280,16 +264,17 @@ class OpenSongImport(SongImport):
                     verse_tag = match.group(1)
                     verse_num = match.group(2)
                     if not len(verse_tag):
-                        verse_tag = u'v'
+                        verse_tag =  VerseType.Tags[VerseType.Verse]
                 else:
                     # Assume it's no.1 if there are no digits
                     verse_tag = verse_def
                     verse_num = u'1'
                 verse_def = u'%s%s' % (verse_tag, verse_num)
-                if verses.has_key(verse_tag) \
+                if verses.has_key(verse_tag) and \
-                    and verses[verse_tag].has_key(verse_num):
+                    verses[verse_tag].has_key(verse_num):
                     self.verse_order_list.append(verse_def)
                 else:
                     log.info(u'Got order %s but not in verse tags, dropping'
                         u'this item from presentation order', verse_def)
-        return True
+        if not self.finish():
+            self.log_error(file.name)

openlp/plugins/songs/lib/sofimport.py

@@ -88,7 +88,6 @@ class SofImport(OooImport):
         paragraphs = self.document.getText().createEnumeration()
         while paragraphs.hasMoreElements():
             if self.stop_import_flag:
-                self.import_wizard.incrementProgressBar(u'Import cancelled', 0)
                 return
             paragraph = paragraphs.nextElement()
             if paragraph.supportsService("com.sun.star.text.Paragraph"):

openlp/plugins/songs/lib/songbeamerimport.py

@@ -33,9 +33,9 @@ import logging
 import os
 import re
 
-from openlp.core.ui.wizard import WizardStrings
 from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
+from openlp.plugins.songs.lib.ui import SongStrings
 
 log = logging.getLogger(__name__)
 
@@ -78,18 +78,18 @@ class SongBeamerImport(SongImport):
         """
         Receive a single file or a list of files to import.
         """
-        if isinstance(self.import_source, list):
+        self.import_wizard.progressBar.setMaximum(len(self.import_source))
-            self.import_wizard.progressBar.setMaximum(
+        if not isinstance(self.import_source, list):
-                len(self.import_source))
+            return
         for file in self.import_source:
             # TODO: check that it is a valid SongBeamer file
+            if self.stop_import_flag:
+                return
             self.set_defaults()
             self.current_verse = u''
             self.current_verse_type = VerseType.Tags[VerseType.Verse]
             read_verses = False
             file_name = os.path.split(file)[1]
-                self.import_wizard.incrementProgressBar(
-                    WizardStrings.ImportingType % file_name, 0)
             if os.path.isfile(file):
                 detect_file = open(file, u'r')
                 details = chardet.detect(detect_file.read(2048))
@@ -98,7 +98,7 @@ class SongBeamerImport(SongImport):
                 songData = infile.readlines()
                 infile.close()
             else:
-                    return False
+                continue
             self.title = file_name.split('.sng')[0]
             read_verses = False
             for line in songData:
@@ -125,11 +125,8 @@ class SongBeamerImport(SongImport):
             if self.current_verse:
                 self.replace_html_tags()
                 self.add_verse(self.current_verse, self.current_verse_type)
-                if self.check_complete():
+            if not self.finish():
-                    self.finish()
+                self.log_error(file)
-                self.import_wizard.incrementProgressBar(
-                    WizardStrings.ImportingType % file_name)
-            return True
 
     def replace_html_tags(self):
         """
@@ -189,7 +186,7 @@ class SongBeamerImport(SongImport):
         elif tag_val[0] == u'#Bible':
             pass
         elif tag_val[0] == u'#Categories':
-            self.topics = line.split(',')
+            self.topics = tag_val[1].split(',')
         elif tag_val[0] == u'#CCLI':
             self.ccli_number = tag_val[1]
         elif tag_val[0] == u'#Chords':
@@ -236,11 +233,12 @@ class SongBeamerImport(SongImport):
             pass
         elif tag_val[0] == u'#Rights':
             song_book_pub = tag_val[1]
-        elif tag_val[0] == u'#Songbook':
+        elif tag_val[0] == u'#Songbook' or tag_val[0] == u'#SongBook':
-            book_num = tag_val[1].split(' / ')
+            book_data = tag_val[1].split(u'/')
-            self.song_book_name = book_num[0]
+            self.song_book_name = book_data[0].strip()
-            if len(book_num) == book_num[1]:
+            if len(book_data) == 2:
-                self.song_number = u''
+                number = book_data[1].strip()
+                self.song_number = number if number.isdigit() else u''
         elif tag_val[0] == u'#Speed':
             pass
         elif tag_val[0] == u'Tempo':
@@ -287,5 +285,4 @@ class SongBeamerImport(SongImport):
                 if marks[1].isdigit():
                     self.current_verse_type += marks[1]
             return True
-        else:
         return False

openlp/plugins/songs/lib/songimport.py

@@ -23,12 +23,14 @@
 # with this program; if not, write to the Free Software Foundation, Inc., 59  #
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
-
 import logging
 import re
+
 from PyQt4 import QtCore
 
-from openlp.core.lib import Receiver, translate
+from openlp.core.lib import Receiver, translate, check_directory_exists
+from openlp.core.ui.wizard import WizardStrings
+from openlp.core.utils import AppLocation
 from openlp.plugins.songs.lib import clean_song, VerseType
 from openlp.plugins.songs.lib.db import Song, Author, Topic, Book, MediaFile
 from openlp.plugins.songs.lib.ui import SongStrings
@@ -66,6 +68,7 @@ class SongImport(QtCore.QObject):
         self.song = None
         self.stop_import_flag = False
         self.set_defaults()
+        self.error_log = []
         QtCore.QObject.connect(Receiver.get_receiver(),
             QtCore.SIGNAL(u'openlp_stop_wizard'), self.stop_import)
 
@@ -94,6 +97,31 @@ class SongImport(QtCore.QObject):
         self.copyright_string = unicode(translate(
             'SongsPlugin.SongImport', 'copyright'))
 
+    def log_error(self, filepath, reason=SongStrings.SongIncomplete):
+        """
+        This should be called, when a song could not be imported.
+
+        ``filepath``
+            This should be the file path if ``self.import_source`` is a list
+            with different files. If it is not a list, but a  single file (for
+            instance a database), then this should be the song's title.
+
+        ``reason``
+            The reason, why the import failed. The string should be as
+            informative as possible.
+        """
+        if self.import_wizard is None:
+            return
+        if self.import_wizard.errorReportTextEdit.isHidden():
+            self.import_wizard.errorReportTextEdit.setText(
+                translate('SongsPlugin.SongImport',
+                'The following songs could not be imported:'))
+            self.import_wizard.errorReportTextEdit.setVisible(True)
+            self.import_wizard.errorCopyToButton.setVisible(True)
+            self.import_wizard.errorSaveToButton.setVisible(True)
+        self.import_wizard.errorReportTextEdit.append(
+            u'- %s (%s)' % (filepath, reason))
+
     def stop_import(self):
         """
         Sets the flag for importers to stop their import
@@ -240,7 +268,7 @@ class SongImport(QtCore.QObject):
         Author not checked here, if no author then "Author unknown" is
         automatically added
         """
-        if self.title == u'' or len(self.verses) == 0:
+        if not self.title or not len(self.verses):
             return False
         else:
             return True
@@ -249,9 +277,15 @@ class SongImport(QtCore.QObject):
         """
         All fields have been set to this song. Write the song to disk.
         """
+        if not self.check_complete():
+            self.set_defaults()
+            return False
         log.info(u'committing song %s to database', self.title)
         song = Song()
         song.title = self.title
+        if self.import_wizard is not None:
+            self.import_wizard.incrementProgressBar(
+                WizardStrings.ImportingType % song.title)
         song.alternate_title = self.alternate_title
         # Values will be set when cleaning the song.
         song.search_title = u''
@@ -308,7 +342,7 @@ class SongImport(QtCore.QObject):
                     publisher=self.song_book_pub)
             song.book = song_book
         for topictext in self.topics:
-            if len(topictext) == 0:
+            if not topictext:
                 continue
             topic = self.manager.get_object_filtered(Topic,
                 Topic.name == topictext)
@@ -318,6 +352,7 @@ class SongImport(QtCore.QObject):
         clean_song(self.manager, song)
         self.manager.save_object(song)
         self.set_defaults()
+        return True
 
     def print_song(self):
         """

openlp/plugins/songs/lib/songshowplusimport.py

@@ -32,6 +32,7 @@ import logging
 import struct
 
 from openlp.core.ui.wizard import WizardStrings
+from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.songimport import SongImport
 
 TITLE = 1
@@ -97,9 +98,9 @@ class SongShowPlusImport(SongImport):
         Receive a single file or a list of files to import.
         """
         if isinstance(self.import_source, list):
+            return
         self.import_wizard.progressBar.setMaximum(len(self.import_source))
         for file in self.import_source:
-                author = u''
             self.sspVerseOrderList = []
             otherCount = 0
             otherList = {}
@@ -107,7 +108,7 @@ class SongShowPlusImport(SongImport):
             self.import_wizard.incrementProgressBar(
                 WizardStrings.ImportingType % file_name, 0)
             songData = open(file, 'rb')
-                while (1):
+            while True:
                 blockKey, = struct.unpack("I", songData.read(4))
                 # The file ends with 4 NUL's
                 if blockKey == 0:
@@ -170,10 +171,8 @@ class SongShowPlusImport(SongImport):
                         % (blockKey, data))
             self.verse_order_list = self.sspVerseOrderList
             songData.close()
-                self.finish()
+            if not self.finish():
-                self.import_wizard.incrementProgressBar(
+                self.log_error(file)
-                    WizardStrings.ImportingType % file_name)
-            return True
 
     def toOpenLPVerseTag(self, verseName, ignoreUnique=False):
         if verseName.find(" ") != -1:
@@ -185,22 +184,19 @@ class SongShowPlusImport(SongImport):
             verseNumber = "1"
         verseType = verseType.lower()
         if verseType == "verse":
-            verseTag = "V"
+            verseTag = VerseType.Tags[VerseType.Verse]
         elif verseType == "chorus":
-            verseTag = "C"
+            verseTag = VerseType.Tags[VerseType.Chorus]
         elif verseType == "bridge":
-            verseTag = "B"
+            verseTag = VerseType.Tags[VerseType.Bridge]
         elif verseType == "pre-chorus":
-            verseTag = "P"
+            verseTag = VerseType.Tags[VerseType.PreChorus]
-        elif verseType == "bridge":
-            verseTag = "B"
         else:
             if not self.otherList.has_key(verseName):
                 if ignoreUnique:
                     return None
                 self.otherCount = self.otherCount + 1
                 self.otherList[verseName] = str(self.otherCount)
-            verseTag = "O"
+            verseTag = VerseType.Tags[VerseType.Other]
             verseNumber = self.otherList[verseName]
-        verseTag = verseTag + verseNumber
+        return verseTag + verseNumber
-        return verseTag

openlp/plugins/songs/lib/ui.py

@@ -40,6 +40,8 @@ class SongStrings(object):
     CopyrightSymbol = translate('OpenLP.Ui', '\xa9', 'Copyright symbol.')
     SongBook = translate('OpenLP.Ui', 'Song Book', 'Singular')
     SongBooks = translate('OpenLP.Ui', 'Song Books', 'Plural')
+    SongIncomplete = translate('OpenLP.Ui','Title and/or verses not found')
     SongMaintenance = translate('OpenLP.Ui', 'Song Maintenance')
     Topic = translate('OpenLP.Ui', 'Topic', 'Singular')
     Topics = translate('OpenLP.Ui', 'Topics', 'Plural')
+    XMLSyntaxError = translate('OpenLP.Ui', 'XML syntax error')

openlp/plugins/songs/lib/wowimport.py

@@ -105,11 +105,7 @@ class WowImport(SongImport):
         if isinstance(self.import_source, list):
             self.import_wizard.progressBar.setMaximum(len(self.import_source))
             for file in self.import_source:
-                author = u''
-                copyright = u''
                 file_name = os.path.split(file)[1]
-                self.import_wizard.incrementProgressBar(
-                    WizardStrings.ImportingType % file_name, 0)
                 # Get the song title
                 self.title = file_name.rpartition(u'.')[0]
                 songData = open(file, 'rb')
@@ -129,7 +125,7 @@ class WowImport(SongImport):
                         self.line_text = unicode(
                             songData.read(ord(songData.read(1))), u'cp1252')
                         songData.seek(1, os.SEEK_CUR)
-                        if block_text != u'':
+                        if block_text:
                             block_text += u'\n'
                         block_text += self.line_text
                         self.lines_to_read -= 1
@@ -138,22 +134,19 @@ class WowImport(SongImport):
                     songData.seek(3, os.SEEK_CUR)
                     # Blocks are seperated by 2 bytes, skip them, but not if
                     # this is the last block!
-                    if (block + 1) < no_of_blocks:
+                    if block + 1 < no_of_blocks:
                         songData.seek(2, os.SEEK_CUR)
                     self.add_verse(block_text, block_type)
                 # Now to extract the author
                 author_length = ord(songData.read(1))
-                if author_length != 0:
+                if author_length:
-                    author = unicode(songData.read(author_length), u'cp1252')
+                    self.parse_author(
+                        unicode(songData.read(author_length), u'cp1252'))
                 # Finally the copyright
                 copyright_length = ord(songData.read(1))
-                if copyright_length != 0:
+                if copyright_length:
-                    copyright = unicode(
+                    self.add_copyright(unicode(
-                        songData.read(copyright_length), u'cp1252')
+                        songData.read(copyright_length), u'cp1252'))
-                self.parse_author(author)
-                self.add_copyright(copyright)
                 songData.close()
-                self.finish()
+                if not self.finish():
-                self.import_wizard.incrementProgressBar(
+                    self.log_error(file)
-                    WizardStrings.ImportingType % file_name)
-            return True