From 21a88085e37b0f1eb5b94dea3c59529115241fdd Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Mon, 30 Apr 2012 00:08:25 +1000 Subject: [PATCH 01/19] First attempt at class openlp.plugins.songs.lib.PowerSongImport --- openlp/core/ui/wizard.py | 1 + openlp/plugins/songs/lib/powersongimport.py | 142 ++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 openlp/plugins/songs/lib/powersongimport.py diff --git a/openlp/core/ui/wizard.py b/openlp/core/ui/wizard.py index 5369c9799..74dcfceaf 100644 --- a/openlp/core/ui/wizard.py +++ b/openlp/core/ui/wizard.py @@ -53,6 +53,7 @@ class WizardStrings(object): OL = u'OpenLyrics' OS = u'OpenSong' OSIS = u'OSIS' + PS = u'PowerSong' SB = u'SongBeamer' SoF = u'Songs of Fellowship' SSP = u'SongShow Plus' diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py new file mode 100644 index 000000000..973f87795 --- /dev/null +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2012 Raoul Snyman # +# Portions copyright (c) 2008-2012 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Michael Gorven, Scott Guerrieri, Matthias Hub, Meinert Jordan, # +# Armin Köhler, Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias # +# Põldaru, Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Frode Woldsund # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +The :mod:`powersongimport` module provides the functionality for importing +PowerSong songs into the OpenLP database. +""" +import logging + +from openlp.core.lib import translate +from openlp.plugins.songs.lib.songimport import SongImport + +log = logging.getLogger(__name__) + +class PowerSongImport(SongImport): + """ + The :class:`PowerSongImport` class provides the ability to import song files + from PowerSong. + + **PowerSong Song File Format:** + + * Encoded as UTF-8. + * The file has a number of fields, with the song metadata fields first, + followed by the lyrics fields. + + Fields: + Each field begins with one of four labels, each of which begin with one + non-printing byte: + + * ``ENQ`` (0x05) ``TITLE`` + * ``ACK`` (0x06) ``AUTHOR`` + * ``CR`` (0x0D) ``COPYRIGHTLINE`` + * ``EOT`` (0x04) ``PART`` + + The field label is separated from the field contents by one random byte. + Each field ends at the next field label, or at the end of the file. + + Metadata fields: + * Every PowerSong file begins with a TITLE field. + * This is followed by zero or more AUTHOR fields. + * The next field is always COPYRIGHTLINE, but it may be empty (in which + case the byte following the label is the null byte 0x00). + When the field contents are not empty, the first byte is 0xC2 and + should be discarded. + This field may contain a CCLI number at the end: e.g. "CCLI 176263" + + Lyrics fields: + * The COPYRIGHTLINE field is followed by zero or more PART fields, each + of which contains one verse. + * Lines have Windows line endings ``CRLF`` (0x0D, 0x0A). + * There is no concept of verse types. + + Valid extensions for a PowerSong song file are: + + * .song + """ + + def __init__(self, manager, **kwargs): + """ + Initialise the PowerSong importer. + """ + SongImport.__init__(self, manager, **kwargs) + + def doImport(self): + """ + Receive a single file or a list of files to import. + """ + if isinstance(self.importSource, list): + self.importWizard.progressBar.setMaximum(len(self.importSource)) + for file in self.importSource: + if self.stopImportFlag: + return + self.setDefaults() + with open(file, 'rb') as song_file: + # Check file is valid PowerSong song format + if song_file.read(6) != u'\x05TITLE': + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', + ('Invalid PowerSong song file. Missing ' + '"\x05TITLE" header.')))) + continue + song_data = song_file.read() + first_part, sep, song_data = song_data.partition( + u'\x0DCOPYRIGHTLINE') + if sep == '': + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', + ('Invalid PowerSong song file. Missing ' + '"\x0DCOPYRIGHTLINE" string.')))) + continue + title_authors = first_part.split(u'\x06AUTHOR') + # Get the song title + self.title = title_authors[0][1:] + # Extract the author(s) + for author in title_authors[1:]: + self.parseAuthor(author[1:]) + # Get copyright and CCLI number + copyright, sep, song_data = song_data.partition( + u'\x04PART') + if sep == '': + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', + ('No verses found. Missing ' + '"\x04PART" string(s).')))) + continue + copyright, sep, ccli_no = copyright[1:].rpartition(u'CCLI ') + if copyright[0] == u'\xC2': + copyright = copyright[1:] + self.addCopyright(copyright) + if ccli_no != '': + ccli_no = ccli_no.strip() + if ccli_no.isdigit(): + self.ccliNumber = ccli_no + # Get the verse(s) + verses = song_data.split(u'\x04PART') + for verse in verses: + self.addVerse(verse[1:]) + if not self.finish(): + self.logError(file) From ea9bfb160d4f6a3bb8f1900c25fbf6e433b5f4fc Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Mon, 30 Apr 2012 16:24:04 +1000 Subject: [PATCH 02/19] Integrated module openlp.plugins.songs.lib.powersongimport --- openlp/plugins/songs/forms/songimportform.py | 43 ++++++++++++++++++++ openlp/plugins/songs/lib/importer.py | 15 ++++--- openlp/plugins/songs/lib/powersongimport.py | 1 + 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 4a44c30ef..686c3d69e 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -171,6 +171,12 @@ class SongImportForm(OpenLPWizard): QtCore.QObject.connect(self.foilPresenterRemoveButton, QtCore.SIGNAL(u'clicked()'), self.onFoilPresenterRemoveButtonClicked) + QtCore.QObject.connect(self.powerSongAddButton, + QtCore.SIGNAL(u'clicked()'), + self.powerSongAddButtonClicked) + QtCore.QObject.connect(self.powerSongRemoveButton, + QtCore.SIGNAL(u'clicked()'), + self.powerSongRemoveButtonClicked) def addCustomPages(self): """ @@ -217,6 +223,8 @@ class SongImportForm(OpenLPWizard): self.addFileSelectItem(u'foilPresenter') # Open Song self.addFileSelectItem(u'openSong', u'OpenSong') + # PowerSong + self.addFileSelectItem(u'powerSong') # SongBeamer self.addFileSelectItem(u'songBeamer') # Song Show Plus @@ -264,6 +272,8 @@ class SongImportForm(OpenLPWizard): self.formatComboBox.setItemText( SongFormat.FoilPresenter, WizardStrings.FP) self.formatComboBox.setItemText(SongFormat.OpenSong, WizardStrings.OS) + self.formatComboBox.setItemText( + SongFormat.PowerSong, WizardStrings.PS) self.formatComboBox.setItemText( SongFormat.SongBeamer, WizardStrings.SB) self.formatComboBox.setItemText( @@ -305,6 +315,10 @@ class SongImportForm(OpenLPWizard): translate('SongsPlugin.ImportWizardForm', 'Add Files...')) self.dreamBeamRemoveButton.setText( translate('SongsPlugin.ImportWizardForm', 'Remove File(s)')) + self.powerSongAddButton.setText( + translate('SongsPlugin.ImportWizardForm', 'Add Files...')) + self.powerSongRemoveButton.setText( + translate('SongsPlugin.ImportWizardForm', 'Remove File(s)')) self.songsOfFellowshipAddButton.setText( translate('SongsPlugin.ImportWizardForm', 'Add Files...')) self.songsOfFellowshipRemoveButton.setText( @@ -417,6 +431,12 @@ class SongImportForm(OpenLPWizard): WizardStrings.YouSpecifyFile % WizardStrings.DB) self.dreamBeamAddButton.setFocus() return False + elif source_format == SongFormat.PowerSong: + if self.powerSongFileListWidget.count() == 0: + critical_error_message_box(UiStrings().NFSp, + WizardStrings.YouSpecifyFile % WizardStrings.PS) + self.powerSongAddButton.setFocus() + return False elif source_format == SongFormat.SongsOfFellowship: if self.songsOfFellowshipFileListWidget.count() == 0: critical_error_message_box(UiStrings().NFSp, @@ -600,6 +620,22 @@ class SongImportForm(OpenLPWizard): """ self.removeSelectedItems(self.dreamBeamFileListWidget) + def onPowerSongAddButtonClicked(self): + """ + Get PowerSong song database files + """ + self.getFiles(WizardStrings.OpenTypeFile % WizardStrings.PS, + self.powerSongFileListWidget, u'%s (*.song)' + % translate('SongsPlugin.ImportWizardForm', + 'PowerSong Song Files') + ) + + def onPowerSongRemoveButtonClicked(self): + """ + Remove selected PowerSong files from the import list + """ + self.removeSelectedItems(self.powerSongFileListWidget) + def onSongsOfFellowshipAddButtonClicked(self): """ Get Songs of Fellowship song database files @@ -717,6 +753,7 @@ class SongImportForm(OpenLPWizard): self.wordsOfWorshipFileListWidget.clear() self.ccliFileListWidget.clear() self.dreamBeamFileListWidget.clear() + self.powerSongFileListWidget.clear() self.songsOfFellowshipFileListWidget.clear() self.genericFileListWidget.clear() self.easySlidesFilenameEdit.setText(u'') @@ -784,6 +821,12 @@ class SongImportForm(OpenLPWizard): filenames=self.getListOfFiles( self.dreamBeamFileListWidget) ) + elif source_format == SongFormat.PowerSong: + # Import PowerSong songs + importer = self.plugin.importSongs(SongFormat.PowerSong, + filenames=self.getListOfFiles( + self.powerSongFileListWidget) + ) elif source_format == SongFormat.SongsOfFellowship: # Import a Songs of Fellowship RTF file importer = self.plugin.importSongs(SongFormat.SongsOfFellowship, diff --git a/openlp/plugins/songs/lib/importer.py b/openlp/plugins/songs/lib/importer.py index 28a57339e..9dde9f0af 100644 --- a/openlp/plugins/songs/lib/importer.py +++ b/openlp/plugins/songs/lib/importer.py @@ -36,6 +36,7 @@ from openlyricsimport import OpenLyricsImport from wowimport import WowImport from cclifileimport import CCLIFileImport from dreambeamimport import DreamBeamImport +from powersongimport import PowerSongImport from ewimport import EasyWorshipSongImport from songbeamerimport import SongBeamerImport from songshowplusimport import SongShowPlusImport @@ -79,11 +80,12 @@ class SongFormat(object): EasyWorship = 7 FoilPresenter = 8 OpenSong = 9 - SongBeamer = 10 - SongShowPlus = 11 - SongsOfFellowship = 12 - WordsOfWorship = 13 - #CSV = 14 + PowerSong = 10 + SongBeamer = 11 + SongShowPlus = 12 + SongsOfFellowship = 13 + WordsOfWorship = 14 + #CSV = 15 @staticmethod def get_class(format): @@ -111,6 +113,8 @@ class SongFormat(object): return CCLIFileImport elif format == SongFormat.DreamBeam: return DreamBeamImport + elif format == SongFormat.PowerSong: + return PowerSongImport elif format == SongFormat.EasySlides: return EasySlidesImport elif format == SongFormat.EasyWorship: @@ -139,6 +143,7 @@ class SongFormat(object): SongFormat.EasyWorship, SongFormat.FoilPresenter, SongFormat.OpenSong, + SongFormat.PowerSong, SongFormat.SongBeamer, SongFormat.SongShowPlus, SongFormat.SongsOfFellowship, diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 973f87795..37d46e35c 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -103,6 +103,7 @@ class PowerSongImport(SongImport): '"\x05TITLE" header.')))) continue song_data = song_file.read() + # Extract title and author fields first_part, sep, song_data = song_data.partition( u'\x0DCOPYRIGHTLINE') if sep == '': From 370603c779e58a83c8e34ce4097c144b0644eb90 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Mon, 30 Apr 2012 20:57:44 +1000 Subject: [PATCH 03/19] PowerSong importer working. Successful on test set of 1057 songs --- openlp/plugins/songs/forms/songimportform.py | 4 +- openlp/plugins/songs/lib/powersongimport.py | 48 ++++++++++++++------ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 686c3d69e..0cacae612 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -173,10 +173,10 @@ class SongImportForm(OpenLPWizard): self.onFoilPresenterRemoveButtonClicked) QtCore.QObject.connect(self.powerSongAddButton, QtCore.SIGNAL(u'clicked()'), - self.powerSongAddButtonClicked) + self.onPowerSongAddButtonClicked) QtCore.QObject.connect(self.powerSongRemoveButton, QtCore.SIGNAL(u'clicked()'), - self.powerSongRemoveButtonClicked) + self.onPowerSongRemoveButtonClicked) def addCustomPages(self): """ diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 37d46e35c..207777570 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -29,6 +29,7 @@ The :mod:`powersongimport` module provides the functionality for importing PowerSong songs into the OpenLP database. """ import logging +import re from openlp.core.lib import translate from openlp.plugins.songs.lib.songimport import SongImport @@ -52,7 +53,7 @@ class PowerSongImport(SongImport): * ``ENQ`` (0x05) ``TITLE`` * ``ACK`` (0x06) ``AUTHOR`` - * ``CR`` (0x0D) ``COPYRIGHTLINE`` + * ``CR`` (0x0d) ``COPYRIGHTLINE`` * ``EOT`` (0x04) ``PART`` The field label is separated from the field contents by one random byte. @@ -63,14 +64,14 @@ class PowerSongImport(SongImport): * This is followed by zero or more AUTHOR fields. * The next field is always COPYRIGHTLINE, but it may be empty (in which case the byte following the label is the null byte 0x00). - When the field contents are not empty, the first byte is 0xC2 and + When the field contents are not empty, the first byte is 0xc2 and should be discarded. This field may contain a CCLI number at the end: e.g. "CCLI 176263" Lyrics fields: * The COPYRIGHTLINE field is followed by zero or more PART fields, each of which contains one verse. - * Lines have Windows line endings ``CRLF`` (0x0D, 0x0A). + * Lines have Windows line endings ``CRLF`` (0x0d, 0x0a). * There is no concept of verse types. Valid extensions for a PowerSong song file are: @@ -102,11 +103,11 @@ class PowerSongImport(SongImport): ('Invalid PowerSong song file. Missing ' '"\x05TITLE" header.')))) continue - song_data = song_file.read() + song_data = unicode(song_file.read(), u'utf-8', u'replace') # Extract title and author fields first_part, sep, song_data = song_data.partition( u'\x0DCOPYRIGHTLINE') - if sep == '': + if not sep: self.logError(file, unicode( translate('SongsPlugin.PowerSongSongImport', ('Invalid PowerSong song file. Missing ' @@ -114,30 +115,47 @@ class PowerSongImport(SongImport): continue title_authors = first_part.split(u'\x06AUTHOR') # Get the song title - self.title = title_authors[0][1:] + self.title = self.stripControlChars(title_authors[0][1:]) # Extract the author(s) for author in title_authors[1:]: - self.parseAuthor(author[1:]) + self.parseAuthor(self.stripControlChars(author[1:])) # Get copyright and CCLI number copyright, sep, song_data = song_data.partition( u'\x04PART') - if sep == '': + if not sep: self.logError(file, unicode( translate('SongsPlugin.PowerSongSongImport', ('No verses found. Missing ' - '"\x04PART" string(s).')))) + '"\x04PART" string.')))) continue copyright, sep, ccli_no = copyright[1:].rpartition(u'CCLI ') - if copyright[0] == u'\xC2': - copyright = copyright[1:] - self.addCopyright(copyright) - if ccli_no != '': + if not sep: + copyright = ccli_no + ccli_no = u'' + if copyright: + if copyright[0] == u'\u00c2': + copyright = copyright[1:] + self.addCopyright(self.stripControlChars( + copyright.rstrip(u'\n'))) + if ccli_no: ccli_no = ccli_no.strip() if ccli_no.isdigit(): - self.ccliNumber = ccli_no + self.ccliNumber = self.stripControlChars(ccli_no) # Get the verse(s) verses = song_data.split(u'\x04PART') for verse in verses: - self.addVerse(verse[1:]) + self.addVerse(self.stripControlChars(verse[1:])) if not self.finish(): self.logError(file) + + def stripControlChars(self, text): + """ + Get rid of ASCII control characters. + + Illegals chars are ASCII code points 0-31 and 127, except: + * ``HT`` (0x09) - Tab + * ``LF`` (0x0a) - Line feed + * ``CR`` (0x0d) - Carriage return + """ + ILLEGAL_CHARS = u'([\x00-\x08\x0b-\x0c\x0e-\x1f\x7f])' + return re.sub(ILLEGAL_CHARS, '', text) \ No newline at end of file From 1184e9219d3d4e4abd9d34e4a18783a90de74db1 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Mon, 30 Apr 2012 22:19:36 +1000 Subject: [PATCH 04/19] Small fixes for comments typos in songs.lib modules --- openlp/plugins/songs/lib/importer.py | 2 +- openlp/plugins/songs/lib/powersongimport.py | 3 +-- openlp/plugins/songs/lib/songimport.py | 2 +- openlp/plugins/songs/lib/wowimport.py | 6 +++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/openlp/plugins/songs/lib/importer.py b/openlp/plugins/songs/lib/importer.py index 9dde9f0af..16d943a73 100644 --- a/openlp/plugins/songs/lib/importer.py +++ b/openlp/plugins/songs/lib/importer.py @@ -90,7 +90,7 @@ class SongFormat(object): @staticmethod def get_class(format): """ - Return the appropriate imeplementation class. + Return the appropriate implementation class. ``format`` The song format. diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 207777570..3c3d9a641 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -75,8 +75,7 @@ class PowerSongImport(SongImport): * There is no concept of verse types. Valid extensions for a PowerSong song file are: - - * .song + * .song """ def __init__(self, manager, **kwargs): diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index 6fd9dd403..b3ceb49ec 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -111,7 +111,7 @@ class SongImport(QtCore.QObject): instance a database), then this should be the song's title. ``reason`` - The reason, why the import failed. The string should be as + The reason why the import failed. The string should be as informative as possible. """ self.setDefaults() diff --git a/openlp/plugins/songs/lib/wowimport.py b/openlp/plugins/songs/lib/wowimport.py index 99f448736..97a11d873 100644 --- a/openlp/plugins/songs/lib/wowimport.py +++ b/openlp/plugins/songs/lib/wowimport.py @@ -71,7 +71,7 @@ class WowImport(SongImport): * ``SOH`` (0x01) - Chorus * ``STX`` (0x02) - Bridge - Blocks are seperated by two bytes. The first byte is 0x01, and the + Blocks are separated by two bytes. The first byte is 0x01, and the second byte is 0x80. Lines: @@ -126,7 +126,7 @@ class WowImport(SongImport): ('Invalid Words of Worship song file. Missing ' '"CSongDoc::CBlock" string.')))) continue - # Seek to the beging of the first block + # Seek to the beginning of the first block song_data.seek(82) for block in range(no_of_blocks): self.linesToRead = ord(song_data.read(4)[:1]) @@ -140,7 +140,7 @@ class WowImport(SongImport): block_text += self.lineText self.linesToRead -= 1 block_type = BLOCK_TYPES[ord(song_data.read(4)[:1])] - # Blocks are seperated by 2 bytes, skip them, but not if + # Blocks are separated by 2 bytes, skip them, but not if # this is the last block! if block + 1 < no_of_blocks: song_data.seek(2, os.SEEK_CUR) From 63b71802abb7f4ff4c450c2dd5aa4e62affd6db6 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Tue, 1 May 2012 23:51:46 +1000 Subject: [PATCH 05/19] Rewrote PowerSongImport class to read variable-length strings directly from file, rather than searching for them. Other minor fixes. --- openlp/plugins/songs/lib/powersongimport.py | 155 ++++++++++---------- openlp/plugins/songs/lib/songimport.py | 2 +- 2 files changed, 77 insertions(+), 80 deletions(-) diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 3c3d9a641..e2ba13f68 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -29,7 +29,6 @@ The :mod:`powersongimport` module provides the functionality for importing PowerSong songs into the OpenLP database. """ import logging -import re from openlp.core.lib import translate from openlp.plugins.songs.lib.songimport import SongImport @@ -43,34 +42,27 @@ class PowerSongImport(SongImport): **PowerSong Song File Format:** - * Encoded as UTF-8. - * The file has a number of fields, with the song metadata fields first, - followed by the lyrics fields. + The file has a number of label-field pairs of variable length. - Fields: - Each field begins with one of four labels, each of which begin with one - non-printing byte: - - * ``ENQ`` (0x05) ``TITLE`` - * ``ACK`` (0x06) ``AUTHOR`` - * ``CR`` (0x0d) ``COPYRIGHTLINE`` - * ``EOT`` (0x04) ``PART`` - - The field label is separated from the field contents by one random byte. - Each field ends at the next field label, or at the end of the file. + Labels and Fields: + * Every label and field is preceded by an integer which specifies its + byte-length. + * If the length < 128 bytes, only one byte is used to encode + the length integer. + * But if it's greater, as many bytes are used as necessary: + * the first byte = (length % 128) + 128 + * the next byte = length / 128 + * another byte is only used if (length / 128) >= 128 + * and so on (3 bytes needed iff length > 16383) Metadata fields: * Every PowerSong file begins with a TITLE field. * This is followed by zero or more AUTHOR fields. - * The next field is always COPYRIGHTLINE, but it may be empty (in which - case the byte following the label is the null byte 0x00). - When the field contents are not empty, the first byte is 0xc2 and - should be discarded. - This field may contain a CCLI number at the end: e.g. "CCLI 176263" + * The next label is always COPYRIGHTLINE, but its field may be empty. + This field may also contain a CCLI number: e.g. "CCLI 176263". Lyrics fields: - * The COPYRIGHTLINE field is followed by zero or more PART fields, each - of which contains one verse. + * Each verse is contained in a PART field. * Lines have Windows line endings ``CRLF`` (0x0d, 0x0a). * There is no concept of verse types. @@ -78,12 +70,6 @@ class PowerSongImport(SongImport): * .song """ - def __init__(self, manager, **kwargs): - """ - Initialise the PowerSong importer. - """ - SongImport.__init__(self, manager, **kwargs) - def doImport(self): """ Receive a single file or a list of files to import. @@ -94,67 +80,78 @@ class PowerSongImport(SongImport): if self.stopImportFlag: return self.setDefaults() - with open(file, 'rb') as song_file: - # Check file is valid PowerSong song format - if song_file.read(6) != u'\x05TITLE': - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', - ('Invalid PowerSong song file. Missing ' - '"\x05TITLE" header.')))) - continue - song_data = unicode(song_file.read(), u'utf-8', u'replace') - # Extract title and author fields - first_part, sep, song_data = song_data.partition( - u'\x0DCOPYRIGHTLINE') - if not sep: + with open(file, 'rb') as self.song_file: + # Get title and check file is valid PowerSong song format + label, field = self.readLabelField() + if label != u'TITLE': self.logError(file, unicode( translate('SongsPlugin.PowerSongSongImport', ('Invalid PowerSong song file. Missing ' - '"\x0DCOPYRIGHTLINE" string.')))) + '"TITLE" header.')))) continue - title_authors = first_part.split(u'\x06AUTHOR') - # Get the song title - self.title = self.stripControlChars(title_authors[0][1:]) - # Extract the author(s) - for author in title_authors[1:]: - self.parseAuthor(self.stripControlChars(author[1:])) - # Get copyright and CCLI number - copyright, sep, song_data = song_data.partition( - u'\x04PART') - if not sep: + else: + self.title = field.replace(u'\n', u' ') + while label: + label, field = self.readLabelField() + # Get the author(s) + if label == u'AUTHOR': + self.parseAuthor(field) + # Get copyright and look for CCLI number + elif label == u'COPYRIGHTLINE': + found_copyright = True + copyright, sep, ccli_no = field.rpartition(u'CCLI') + if not sep: + copyright = ccli_no + ccli_no = u'' + if copyright: + self.addCopyright(copyright.rstrip( + u'\n').replace(u'\n', u' ')) + if ccli_no: + ccli_no = ccli_no.strip(u' :') + if ccli_no.isdigit(): + self.ccliNumber = ccli_no + # Get verse(s) + elif label == u'PART': + self.addVerse(field) + # Check for copyright label + if not found_copyright: self.logError(file, unicode( translate('SongsPlugin.PowerSongSongImport', - ('No verses found. Missing ' - '"\x04PART" string.')))) + ('"%s" Invalid PowerSong song file. Missing ' + '"COPYRIGHTLINE" string.' % self.title)))) + continue + # Check for at least one verse + if not self.verses: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', + ('"%s" No verses found. Missing "PART" string.' + % self.title)))) continue - copyright, sep, ccli_no = copyright[1:].rpartition(u'CCLI ') - if not sep: - copyright = ccli_no - ccli_no = u'' - if copyright: - if copyright[0] == u'\u00c2': - copyright = copyright[1:] - self.addCopyright(self.stripControlChars( - copyright.rstrip(u'\n'))) - if ccli_no: - ccli_no = ccli_no.strip() - if ccli_no.isdigit(): - self.ccliNumber = self.stripControlChars(ccli_no) - # Get the verse(s) - verses = song_data.split(u'\x04PART') - for verse in verses: - self.addVerse(self.stripControlChars(verse[1:])) if not self.finish(): self.logError(file) - def stripControlChars(self, text): + def readLabelField(self): """ - Get rid of ASCII control characters. + Return as a 2-tuple the next two variable-length strings from song file + """ + label = unicode(self.song_file.read( + self.readLength()), u'utf-8', u'ignore') + if label: + field = unicode(self.song_file.read( + self.readLength()), u'utf-8', u'ignore') + else: + field = u'' + return label, field - Illegals chars are ASCII code points 0-31 and 127, except: - * ``HT`` (0x09) - Tab - * ``LF`` (0x0a) - Line feed - * ``CR`` (0x0d) - Carriage return + def readLength(self): """ - ILLEGAL_CHARS = u'([\x00-\x08\x0b-\x0c\x0e-\x1f\x7f])' - return re.sub(ILLEGAL_CHARS, '', text) \ No newline at end of file + Return the byte-length of the next variable-length string in song file + """ + this_byte_char = self.song_file.read(1) + if not this_byte_char: + return 0 + this_byte = ord(this_byte_char) + if this_byte < 128: + return this_byte + else: + return (self.readLength() * 128) + (this_byte - 128) diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index b3ceb49ec..79e960919 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -107,7 +107,7 @@ class SongImport(QtCore.QObject): ``filepath`` This should be the file path if ``self.importSource`` is a list - with different files. If it is not a list, but a single file (for + 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`` From 8877484aea388bd84ac786ab9f9ed1c44271c660 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Wed, 2 May 2012 19:14:30 +1000 Subject: [PATCH 06/19] Tidy up code, stonger error checking. --- openlp/plugins/songs/lib/powersongimport.py | 139 +++++++++++--------- 1 file changed, 78 insertions(+), 61 deletions(-) diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index e2ba13f68..1b99d756f 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -72,67 +72,67 @@ class PowerSongImport(SongImport): def doImport(self): """ - Receive a single file or a list of files to import. + Receive a list of files to import. """ - if isinstance(self.importSource, list): - self.importWizard.progressBar.setMaximum(len(self.importSource)) - for file in self.importSource: - if self.stopImportFlag: - return - self.setDefaults() - with open(file, 'rb') as self.song_file: - # Get title and check file is valid PowerSong song format + if not isinstance(self.importSource, list): + return + self.importWizard.progressBar.setMaximum(len(self.importSource)) + for file in self.importSource: + if self.stopImportFlag: + return + self.setDefaults() + parse_error = False + with open(file, 'rb') as self.song_file: + # Get title to check file is valid PowerSong song format + label, field = self.readLabelField() + if label == u'TITLE': + self.title = field.replace(u'\n', u' ') + else: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', \ + 'Invalid PowerSong file. Missing "TITLE" header.'))) + continue + # Get rest of fields from file + while True: label, field = self.readLabelField() - if label != u'TITLE': - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', - ('Invalid PowerSong song file. Missing ' - '"TITLE" header.')))) - continue + if not label: + break + if label == u'AUTHOR': + self.parseAuthor(field) + elif label == u'COPYRIGHTLINE': + found_copyright = True + self.parseCopyrightCCLI(field) + elif label == u'PART': + self.addVerse(field) else: - self.title = field.replace(u'\n', u' ') - while label: - label, field = self.readLabelField() - # Get the author(s) - if label == u'AUTHOR': - self.parseAuthor(field) - # Get copyright and look for CCLI number - elif label == u'COPYRIGHTLINE': - found_copyright = True - copyright, sep, ccli_no = field.rpartition(u'CCLI') - if not sep: - copyright = ccli_no - ccli_no = u'' - if copyright: - self.addCopyright(copyright.rstrip( - u'\n').replace(u'\n', u' ')) - if ccli_no: - ccli_no = ccli_no.strip(u' :') - if ccli_no.isdigit(): - self.ccliNumber = ccli_no - # Get verse(s) - elif label == u'PART': - self.addVerse(field) - # Check for copyright label - if not found_copyright: + parse_error = True self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', - ('"%s" Invalid PowerSong song file. Missing ' - '"COPYRIGHTLINE" string.' % self.title)))) - continue - # Check for at least one verse - if not self.verses: - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', - ('"%s" No verses found. Missing "PART" string.' - % self.title)))) - continue - if not self.finish(): - self.logError(file) + translate('SongsPlugin.PowerSongSongImport', \ + '"%s" Invalid PowerSong file. Unknown header: "%s".' + % (self.title, label)))) + break + if parse_error: + continue + # Check that file had COPYRIGHTLINE label + if not found_copyright: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', \ + '"%s" Invalid PowerSong file. Missing "COPYRIGHTLINE" \ + header.' % self.title))) + continue + # Check that file had at least one verse + if not self.verses: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongSongImport', \ + '"%s" Verses not found. Missing "PART" header.' + % self.title))) + continue + if not self.finish(): + self.logError(file) def readLabelField(self): """ - Return as a 2-tuple the next two variable-length strings from song file + Read (as a 2-tuple) the next two variable-length strings """ label = unicode(self.song_file.read( self.readLength()), u'utf-8', u'ignore') @@ -145,13 +145,30 @@ class PowerSongImport(SongImport): def readLength(self): """ - Return the byte-length of the next variable-length string in song file + Read the byte-length of the next variable-length string + + If at the end of the file, returns 0. """ - this_byte_char = self.song_file.read(1) - if not this_byte_char: + this_byte = self.song_file.read(1) + if not this_byte: return 0 - this_byte = ord(this_byte_char) - if this_byte < 128: - return this_byte + this_byte_val = ord(this_byte) + if this_byte_val < 128: + return this_byte_val else: - return (self.readLength() * 128) + (this_byte - 128) + return (self.readLength() * 128) + (this_byte_val - 128) + + def parseCopyrightCCLI(self, field): + """ + Look for CCLI song number, and get copyright + """ + copyright, sep, ccli_no = field.rpartition(u'CCLI') + if not sep: + copyright = ccli_no + ccli_no = u'' + if copyright: + self.addCopyright(copyright.rstrip(u'\n').replace(u'\n', u' ')) + if ccli_no: + ccli_no = ccli_no.strip(u' :') + if ccli_no.isdigit(): + self.ccliNumber = ccli_no From 45c180308a5103ed03ed344060ae34522159bb33 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Thu, 3 May 2012 22:41:49 +1000 Subject: [PATCH 07/19] Implemented BinaryReader.Read7BitEncodedInt from .NET. Tidy code. --- openlp/plugins/songs/lib/powersongimport.py | 171 +++++++++++--------- 1 file changed, 96 insertions(+), 75 deletions(-) diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 1b99d756f..9d5fa8f8e 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -42,31 +42,30 @@ class PowerSongImport(SongImport): **PowerSong Song File Format:** - The file has a number of label-field pairs of variable length. + The file has a number of label-field pairs. - Labels and Fields: - * Every label and field is preceded by an integer which specifies its - byte-length. - * If the length < 128 bytes, only one byte is used to encode - the length integer. - * But if it's greater, as many bytes are used as necessary: - * the first byte = (length % 128) + 128 - * the next byte = length / 128 - * another byte is only used if (length / 128) >= 128 - * and so on (3 bytes needed iff length > 16383) + Label and Field strings: + + * Every label and field is a variable length string preceded by an + integer specifying it's byte length. + * Integer is 32-bit but is encoded in 7-bit format to save space. Thus + if length will fit in 7 bits (ie <= 127) it takes up only one byte. Metadata fields: - * Every PowerSong file begins with a TITLE field. - * This is followed by zero or more AUTHOR fields. - * The next label is always COPYRIGHTLINE, but its field may be empty. + + * Every PowerSong file has a TITLE field. + * There is zero or more AUTHOR fields. + * There is always a COPYRIGHTLINE label, but its field may be empty. This field may also contain a CCLI number: e.g. "CCLI 176263". Lyrics fields: + * Each verse is contained in a PART field. * Lines have Windows line endings ``CRLF`` (0x0d, 0x0a). * There is no concept of verse types. Valid extensions for a PowerSong song file are: + * .song """ @@ -75,6 +74,8 @@ class PowerSongImport(SongImport): Receive a list of files to import. """ if not isinstance(self.importSource, list): + self.logError(unicode(translate('SongsPlugin.PowerSongImport', + 'No files to import.'))) return self.importWizard.progressBar.setMaximum(len(self.importSource)) for file in self.importSource: @@ -82,83 +83,103 @@ class PowerSongImport(SongImport): return self.setDefaults() parse_error = False - with open(file, 'rb') as self.song_file: - # Get title to check file is valid PowerSong song format - label, field = self.readLabelField() - if label == u'TITLE': - self.title = field.replace(u'\n', u' ') - else: - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', \ - 'Invalid PowerSong file. Missing "TITLE" header.'))) - continue - # Get rest of fields from file + with open(file, 'rb') as song_data: while True: - label, field = self.readLabelField() - if not label: - break - if label == u'AUTHOR': - self.parseAuthor(field) - elif label == u'COPYRIGHTLINE': - found_copyright = True - self.parseCopyrightCCLI(field) - elif label == u'PART': - self.addVerse(field) - else: + try: + label = self._readString(song_data) + if not label: + break + field = self._readString(song_data) + except ValueError: parse_error = True self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', \ - '"%s" Invalid PowerSong file. Unknown header: "%s".' - % (self.title, label)))) + translate('SongsPlugin.PowerSongImport', + 'Invalid PowerSong file. Unexpected byte value.'))) break - if parse_error: - continue - # Check that file had COPYRIGHTLINE label - if not found_copyright: - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', \ - '"%s" Invalid PowerSong file. Missing "COPYRIGHTLINE" \ - header.' % self.title))) - continue - # Check that file had at least one verse - if not self.verses: - self.logError(file, unicode( - translate('SongsPlugin.PowerSongSongImport', \ - '"%s" Verses not found. Missing "PART" header.' - % self.title))) - continue + else: + if label == u'TITLE': + self.title = field.replace(u'\n', u' ') + elif label == u'AUTHOR': + self.parseAuthor(field) + elif label == u'COPYRIGHTLINE': + found_copyright = True + self._parseCopyrightCCLI(field) + elif label == u'PART': + self.addVerse(field) + if parse_error: + continue + # Check that file had TITLE field + if not self.title: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongImport', + 'Invalid PowerSong file. Missing "TITLE" header.'))) + continue + # Check that file had COPYRIGHTLINE label + if not found_copyright: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongImport', + '"%s" Invalid PowerSong file. Missing "COPYRIGHTLINE" ' + 'header.' % self.title))) + continue + # Check that file had at least one verse + if not self.verses: + self.logError(file, unicode( + translate('SongsPlugin.PowerSongImport', + '"%s" Verses not found. Missing "PART" header.' + % self.title))) + continue if not self.finish(): self.logError(file) - def readLabelField(self): + def _readString(self, file_object): """ - Read (as a 2-tuple) the next two variable-length strings + Reads in next variable-length string. """ - label = unicode(self.song_file.read( - self.readLength()), u'utf-8', u'ignore') - if label: - field = unicode(self.song_file.read( - self.readLength()), u'utf-8', u'ignore') - else: - field = u'' - return label, field + string_len = self._read7BitEncodedInteger(file_object) + return unicode(file_object.read(string_len), u'utf-8', u'ignore') - def readLength(self): + def _read7BitEncodedInteger(self, file_object): """ - Read the byte-length of the next variable-length string + Reads in a 32-bit integer in compressed 7-bit format. - If at the end of the file, returns 0. + Accomplished by reading the integer 7 bits at a time. The high bit + of the byte when set means to continue reading more bytes. + If the integer will fit in 7 bits (ie <= 127), it only takes up one + byte. Otherwise, it may take up to 5 bytes. + + Reference: .NET method System.IO.BinaryReader.Read7BitEncodedInt """ - this_byte = self.song_file.read(1) - if not this_byte: + val = 0 + shift = 0 + i = 0 + while True: + # Check for corrupted stream (since max 5 bytes per 32-bit integer) + if i == 5: + raise ValueError + byte = self._readByte(file_object) + # Strip high bit and shift left + val += (byte & 0x7f) << shift + shift += 7 + high_bit_set = byte & 0x80 + if not high_bit_set: + break + i += 1 + return val + + def _readByte(self, file_object): + """ + Reads in next byte as an unsigned integer + + Note: returns 0 at end of file. + """ + byte_str = file_object.read(1) + # If read result is empty, then reached end of file + if not byte_str: return 0 - this_byte_val = ord(this_byte) - if this_byte_val < 128: - return this_byte_val else: - return (self.readLength() * 128) + (this_byte_val - 128) + return ord(byte_str) - def parseCopyrightCCLI(self, field): + def _parseCopyrightCCLI(self, field): """ Look for CCLI song number, and get copyright """ From 416cbe465ea9465b953093065a50fa69ea75f450 Mon Sep 17 00:00:00 2001 From: Samuel Findlay Date: Thu, 3 May 2012 22:50:10 +1000 Subject: [PATCH 08/19] Changed 'PowerSong' to 'PowerSong 1.0' --- openlp/core/ui/wizard.py | 2 +- openlp/plugins/songs/forms/songimportform.py | 2 +- openlp/plugins/songs/lib/powersongimport.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/wizard.py b/openlp/core/ui/wizard.py index 74dcfceaf..500d958fd 100644 --- a/openlp/core/ui/wizard.py +++ b/openlp/core/ui/wizard.py @@ -53,7 +53,7 @@ class WizardStrings(object): OL = u'OpenLyrics' OS = u'OpenSong' OSIS = u'OSIS' - PS = u'PowerSong' + PS = u'PowerSong 1.0' SB = u'SongBeamer' SoF = u'Songs of Fellowship' SSP = u'SongShow Plus' diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 0cacae612..d5f7715ea 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -627,7 +627,7 @@ class SongImportForm(OpenLPWizard): self.getFiles(WizardStrings.OpenTypeFile % WizardStrings.PS, self.powerSongFileListWidget, u'%s (*.song)' % translate('SongsPlugin.ImportWizardForm', - 'PowerSong Song Files') + 'PowerSong 1.0 Song Files') ) def onPowerSongRemoveButtonClicked(self): diff --git a/openlp/plugins/songs/lib/powersongimport.py b/openlp/plugins/songs/lib/powersongimport.py index 9d5fa8f8e..31491398c 100644 --- a/openlp/plugins/songs/lib/powersongimport.py +++ b/openlp/plugins/songs/lib/powersongimport.py @@ -40,9 +40,9 @@ class PowerSongImport(SongImport): The :class:`PowerSongImport` class provides the ability to import song files from PowerSong. - **PowerSong Song File Format:** + **PowerSong 1.0 Song File Format:** - The file has a number of label-field pairs. + The file has a number of label-field (think key-value) pairs. Label and Field strings: From 785787d7f3e0a1a83fdb23dbbc0ce271d90c43d5 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 14:23:42 +0200 Subject: [PATCH 09/19] added secondary criterion for the image queue to privilege images which were added earlier over images which were added later, when both have the same priority --- openlp/core/lib/imagemanager.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index 0139cd001..660c3fe22 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -100,6 +100,7 @@ class Image(object): variables ``image`` and ``image_bytes`` to ``None`` and add the image object to the queue of images to process. """ + NUMBER = 0 def __init__(self, name, path, source, background): self.name = name self.path = path @@ -108,11 +109,24 @@ class Image(object): self.priority = Priority.Normal self.source = source self.background = background + self._number = Image.NUMBER + Image.NUMBER += 1 class PriorityQueue(Queue.PriorityQueue): """ Customised ``Queue.PriorityQueue``. + + Each item in the queue must be tuple with three values. The fist value + is the priority, the second value the image's ``_number`` attribute. The + last value the :class:`image` instance itself:: + + (Priority.Normal, image._number, image) + + Doing this, the :class:`Queue.PriorityQueue` will sort the images according + to their priorities, but also according to there number. However, the number + only has an impact on the result if there are more images with the same + priority. In such case the image which has been added earlier is privileged. """ def modify_priority(self, image, new_priority): """ @@ -126,7 +140,7 @@ class PriorityQueue(Queue.PriorityQueue): """ self.remove(image) image.priority = new_priority - self.put((image.priority, image)) + self.put((image.priority, image._number, image)) def remove(self, image): """ @@ -135,8 +149,8 @@ class PriorityQueue(Queue.PriorityQueue): ``image`` The image to remove. This should be an ``Image`` instance. """ - if (image.priority, image) in self.queue: - self.queue.remove((image.priority, image)) + if (image.priority, image._number, image) in self.queue: + self.queue.remove((image.priority, image._number, image)) class ImageManager(QtCore.QObject): @@ -261,7 +275,7 @@ class ImageManager(QtCore.QObject): if not name in self._cache: image = Image(name, path, source, background) self._cache[name] = image - self._conversion_queue.put((image.priority, image)) + self._conversion_queue.put((image.priority, image._number, image)) else: log.debug(u'Image in cache %s:%s' % (name, path)) # We want only one thread. @@ -282,7 +296,7 @@ class ImageManager(QtCore.QObject): Actually does the work. """ log.debug(u'_process_cache') - image = self._conversion_queue.get()[1] + image = self._conversion_queue.get()[2] # Generate the QImage for the image. if image.image is None: image.image = resize_image(image.path, self.width, self.height, From aa917e7aba45f1c6fc226b5f7270b84b7d68b6b6 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 14:38:06 +0200 Subject: [PATCH 10/19] fixed cap --- openlp/core/lib/imagemanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index 660c3fe22..9ef02a9a6 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -119,7 +119,7 @@ class PriorityQueue(Queue.PriorityQueue): Each item in the queue must be tuple with three values. The fist value is the priority, the second value the image's ``_number`` attribute. The - last value the :class:`image` instance itself:: + last value the :class:`Image` instance itself:: (Priority.Normal, image._number, image) From 7f7ac0c54e9f9f429ec93c9140c03a2a600d062b Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 14:40:31 +0200 Subject: [PATCH 11/19] extended doc string --- openlp/core/lib/imagemanager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index 9ef02a9a6..10cfce616 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -133,10 +133,11 @@ class PriorityQueue(Queue.PriorityQueue): Modifies the priority of the given ``image``. ``image`` - The image to remove. This should be an ``Image`` instance. + The image to remove. This should be an :class:`Image` instance. ``new_priority`` - The image's new priority. + The image's new priority. See the :class:`Priority` class for + priorities. """ self.remove(image) image.priority = new_priority From 34b317398888e7bddcc291f06bf8aaef6f140b70 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 15:59:04 +0200 Subject: [PATCH 12/19] fixed bug 885874 (Song with mis matched formatting tags abends on render) Fixes: https://launchpad.net/bugs/885874 --- openlp/core/lib/renderer.py | 12 ++++---- openlp/plugins/songs/lib/xml.py | 50 +++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/openlp/core/lib/renderer.py b/openlp/core/lib/renderer.py index 39f69dda6..bc10bac95 100644 --- a/openlp/core/lib/renderer.py +++ b/openlp/core/lib/renderer.py @@ -236,8 +236,8 @@ class Renderer(object): # the first two slides (and neglect the last for now). if len(slides) == 3: html_text = expand_tags(u'\n'.join(slides[:2])) - # We check both slides to determine if the optional break is - # needed (there is only one optional break). + # We check both slides to determine if the optional split is + # needed (there is only one optional split). else: html_text = expand_tags(u'\n'.join(slides)) html_text = html_text.replace(u'\n', u'
') @@ -248,8 +248,8 @@ class Renderer(object): else: # The first optional slide fits, which means we have to # render the first optional slide. - text_contains_break = u'[---]' in text - if text_contains_break: + text_contains_split = u'[---]' in text + if text_contains_split: try: text_to_render, text = \ text.split(u'\n[---]\n', 1) @@ -264,7 +264,7 @@ class Renderer(object): if len(slides) > 1 and text: # Add all slides apart from the last one the list. pages.extend(slides[:-1]) - if text_contains_break: + if text_contains_split: text = slides[-1] + u'\n[---]\n' + text else: text = slides[-1] + u'\n'+ text @@ -493,7 +493,7 @@ class Renderer(object): (raw_text.find(tag[u'start tag']), tag[u'start tag'], tag[u'end tag'])) html_tags.append( - (raw_text.find(tag[u'start tag']), tag[u'start html'])) + (raw_text.find(tag[u'start tag']), tag[u'start html'])) # Sort the lists, so that the tags which were opened first on the first # slide (the text we are checking) will be opened first on the next # slide as well. diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index 816742d11..f16833ac4 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -335,17 +335,57 @@ class OpenLyrics(object): if u'lang' in verse[0]: verse_element.set(u'lang', verse[0][u'lang']) # Create a list with all "virtual" verses. - virtual_verses = cgi.escape(verse[1]) - virtual_verses = virtual_verses.split(u'[---]') - for index, virtual_verse in enumerate(virtual_verses): + optional_verses = cgi.escape(verse[1]) + optional_verses = optional_verses.split(u'[---]') + start_tags = u'' + end_tags = u'' + for index, optional_verse in enumerate(optional_verses): + optional_verse = start_tags + optional_verse + start_tags, end_tags = self._get_start_tags(optional_verse) + optional_verse += end_tags # Add formatting tags to text lines_element = self._add_text_with_tags_to_lines(verse_element, - virtual_verse, tags_element) + optional_verse, tags_element) # Do not add the break attribute to the last lines element. - if index < len(virtual_verses) - 1: + if index < len(optional_verses) - 1: lines_element.set(u'break', u'optional') return self._extract_xml(song_xml) + def _get_start_tags(self, text): + """ + Tests the given text for not closed formatting tags and returns a tuple + consisting of two unicode strings:: + + (u'{st}{r}', u'{/r}{/st}') + + The first unicode string are the start tags (for the next slide). The + second unicode string are the end tags. + + ``text`` + The text to test. The text must **not** contain html tags, only + OpenLP formatting tags are allowed:: + + {st}{r}Text text text + """ + tags = [] + for tag in FormattingTags.get_html_tags(): + if tag[u'start tag'] == u'{br}': + continue + if text.count(tag[u'start tag']) != text.count(tag[u'end tag']): + tags.append((text.find(tag[u'start tag']), + tag[u'start tag'], tag[u'end tag'])) + # Sort the lists, so that the tags which were opened first on the first + # slide (the text we are checking) will be opened first on the next + # slide as well. + tags.sort(key=lambda tag: tag[0]) + end_tags = [] + start_tags = [] + for tag in tags: + start_tags.append(tag[1]) + end_tags.append(tag[2]) + end_tags.reverse() + return u''.join(start_tags), u''.join(end_tags) + def xml_to_song(self, xml, parse_and_temporary_save=False): """ Create and save a song from OpenLyrics format xml to the database. Since From 75f1041d27e97f0c89d9b8e53c402f231906078b Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 16:03:48 +0200 Subject: [PATCH 13/19] removed occurrences of 'virtual' split --- openlp/plugins/songs/lib/xml.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index e5f5f7ff7..17a8db91e 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -33,7 +33,7 @@ The basic XML for storing the lyrics in the song database looks like this:: - + @@ -135,7 +135,7 @@ class SongXML(object): The returned list has the following format:: [[{'type': 'v', 'label': '1'}, - u"virtual slide 1[---]virtual slide 2"], + u"optional slide split 1[---]optional slide split 2"], [{'lang': 'en', 'type': 'c', 'label': '1'}, u"English chorus"]] """ self.song_xml = None @@ -334,7 +334,7 @@ class OpenLyrics(object): self._add_text_to_element(u'verse', lyrics, None, verse_def) if u'lang' in verse[0]: verse_element.set(u'lang', verse[0][u'lang']) - # Create a list with all "virtual" verses. + # Create a list with all "optional" verses. optional_verses = cgi.escape(verse[1]) optional_verses = optional_verses.split(u'[---]') start_tags = u'' From 74ff2f92ee9bc3437b36fc3cbc299d3e85f011c4 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 16:08:07 +0200 Subject: [PATCH 14/19] added comment --- openlp/plugins/songs/lib/xml.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index 17a8db91e..2e88a52a2 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -340,6 +340,7 @@ class OpenLyrics(object): start_tags = u'' end_tags = u'' for index, optional_verse in enumerate(optional_verses): + # Fix up missing end and start tags such as {r} or {/r}. optional_verse = start_tags + optional_verse start_tags, end_tags = self._get_start_tags(optional_verse) optional_verse += end_tags From f658c741ebdfc6ed7e346b737a4bd9ba6c2b569b Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 16:12:11 +0200 Subject: [PATCH 15/19] renamed method --- openlp/plugins/songs/lib/xml.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index 2e88a52a2..abc5a6dbe 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -342,7 +342,7 @@ class OpenLyrics(object): for index, optional_verse in enumerate(optional_verses): # Fix up missing end and start tags such as {r} or {/r}. optional_verse = start_tags + optional_verse - start_tags, end_tags = self._get_start_tags(optional_verse) + start_tags, end_tags = self._get_missing_tags(optional_verse) optional_verse += end_tags # Add formatting tags to text lines_element = self._add_text_with_tags_to_lines(verse_element, @@ -352,7 +352,7 @@ class OpenLyrics(object): lines_element.set(u'break', u'optional') return self._extract_xml(song_xml) - def _get_start_tags(self, text): + def _get_missing_tags(self, text): """ Tests the given text for not closed formatting tags and returns a tuple consisting of two unicode strings:: From b3f733233ab5a444a1fd34114363dd1694a32c30 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 16:25:55 +0200 Subject: [PATCH 16/19] fixed two extra new lines being added --- openlp/plugins/songs/lib/xml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index abc5a6dbe..1e35d07ad 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -336,7 +336,7 @@ class OpenLyrics(object): verse_element.set(u'lang', verse[0][u'lang']) # Create a list with all "optional" verses. optional_verses = cgi.escape(verse[1]) - optional_verses = optional_verses.split(u'[---]') + optional_verses = optional_verses.split(u'\n[---]\n') start_tags = u'' end_tags = u'' for index, optional_verse in enumerate(optional_verses): From b72101c7247401885e8f59c3e36539f35e112778 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 16:36:47 +0200 Subject: [PATCH 17/19] fixed formatting tag not being present on slides --- openlp/core/lib/renderer.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openlp/core/lib/renderer.py b/openlp/core/lib/renderer.py index 78e9324bc..30a02e94a 100644 --- a/openlp/core/lib/renderer.py +++ b/openlp/core/lib/renderer.py @@ -255,6 +255,10 @@ class Renderer(object): except: text_to_render = text.split(u'\n[---]\n')[0] text = u'' + text_to_render, raw_tags, html_tags = \ + self._get_start_tags(text_to_render) + if text: + text = raw_tags + text else: text_to_render = text text = u'' From 5959ee5158fc87ee3a82474320454f604ec2b17a Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 18:03:40 +0200 Subject: [PATCH 18/19] fixed regression, reworked staticmethods, changed button behaviour to reflect the internal actions --- openlp/core/lib/formattingtags.py | 69 +++++++++++------------------ openlp/core/ui/formattingtagform.py | 42 ++++++++++++------ openlp/plugins/songs/lib/xml.py | 7 ++- 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/openlp/core/lib/formattingtags.py b/openlp/core/lib/formattingtags.py index bec2db63a..11fd898c3 100644 --- a/openlp/core/lib/formattingtags.py +++ b/openlp/core/lib/formattingtags.py @@ -46,13 +46,36 @@ class FormattingTags(object): """ Provide access to the html_expands list. """ - # Load user defined tags otherwise user defined tags are not present. return FormattingTags.html_expands @staticmethod - def reset_html_tags(): + def save_html_tags(): """ - Resets the html_expands list. + Saves all formatting tags except protected ones. + """ + tags = [] + for tag in FormattingTags.html_expands: + if not tag[u'protected'] and not tag.get(u'temporary'): + # Using dict ensures that copy is made and encoding of values + # a little later does not affect tags in the original list + tags.append(dict(tag)) + tag = tags[-1] + # Remove key 'temporary' from tags. + # It is not needed to be saved. + if u'temporary' in tag: + del tag[u'temporary'] + for element in tag: + if isinstance(tag[element], unicode): + tag[element] = tag[element].encode('utf8') + # Formatting Tags were also known as display tags. + QtCore.QSettings().setValue(u'displayTags/html_tags', + QtCore.QVariant(cPickle.dumps(tags) if tags else u'')) + + @staticmethod + def load_tags(): + """ + Load the Tags from store so can be used in the system or used to + update the display. """ temporary_tags = [tag for tag in FormattingTags.html_expands if tag.get(u'temporary')] @@ -140,38 +163,6 @@ class FormattingTags(object): FormattingTags.add_html_tags(base_tags) FormattingTags.add_html_tags(temporary_tags) - @staticmethod - def save_html_tags(): - """ - Saves all formatting tags except protected ones. - """ - tags = [] - for tag in FormattingTags.html_expands: - if not tag[u'protected'] and not tag.get(u'temporary'): - # Using dict ensures that copy is made and encoding of values - # a little later does not affect tags in the original list - tags.append(dict(tag)) - tag = tags[-1] - # Remove key 'temporary' from tags. - # It is not needed to be saved. - if u'temporary' in tag: - del tag[u'temporary'] - for element in tag: - if isinstance(tag[element], unicode): - tag[element] = tag[element].encode('utf8') - # Formatting Tags were also known as display tags. - QtCore.QSettings().setValue(u'displayTags/html_tags', - QtCore.QVariant(cPickle.dumps(tags) if tags else u'')) - - @staticmethod - def load_tags(): - """ - Load the Tags from store so can be used in the system or used to - update the display. If Cancel was selected this is needed to reset the - dsiplay to the correct version. - """ - # Initial Load of the Tags - FormattingTags.reset_html_tags() # Formatting Tags were also known as display tags. user_expands = QtCore.QSettings().value(u'displayTags/html_tags', QtCore.QVariant(u'')).toString() @@ -187,17 +178,13 @@ class FormattingTags(object): FormattingTags.add_html_tags(user_tags) @staticmethod - def add_html_tags(tags, save=False): + def add_html_tags(tags): """ Add a list of tags to the list. ``tags`` The list with tags to add. - ``save`` - Defaults to ``False``. If set to ``True`` the given ``tags`` are - saved to the config. - Each **tag** has to be a ``dict`` and should have the following keys: * desc @@ -225,8 +212,6 @@ class FormattingTags(object): displaying text containing the tag. It has to be a ``boolean``. """ FormattingTags.html_expands.extend(tags) - if save: - FormattingTags.save_html_tags() @staticmethod def remove_html_tag(tag_id): diff --git a/openlp/core/ui/formattingtagform.py b/openlp/core/ui/formattingtagform.py index d6f880e3f..1084d6a3d 100644 --- a/openlp/core/ui/formattingtagform.py +++ b/openlp/core/ui/formattingtagform.py @@ -57,6 +57,14 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): QtCore.SIGNAL(u'clicked()'), self.onDeleteClicked) QtCore.QObject.connect(self.buttonBox, QtCore.SIGNAL(u'rejected()'), self.close) + QtCore.QObject.connect(self.descriptionLineEdit, + QtCore.SIGNAL(u'textEdited(QString)'), self.onTextEdited) + QtCore.QObject.connect(self.tagLineEdit, + QtCore.SIGNAL(u'textEdited(QString)'), self.onTextEdited) + QtCore.QObject.connect(self.startTagLineEdit, + QtCore.SIGNAL(u'textEdited(QString)'), self.onTextEdited) + QtCore.QObject.connect(self.endTagLineEdit, + QtCore.SIGNAL(u'textEdited(QString)'), self.onTextEdited) # Forces reloading of tags from openlp configuration. FormattingTags.load_tags() @@ -65,7 +73,7 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): Load Display and set field state. """ # Create initial copy from master - self._resetTable() + self._reloadTable() self.selected = -1 return QtGui.QDialog.exec_(self) @@ -73,9 +81,9 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): """ Table Row selected so display items and set field state. """ - row = self.tagTableWidget.currentRow() - html = FormattingTags.html_expands[row] - self.selected = row + self.savePushButton.setEnabled(False) + self.selected = self.tagTableWidget.currentRow() + html = FormattingTags.get_html_tags()[self.selected] self.descriptionLineEdit.setText(html[u'desc']) self.tagLineEdit.setText(self._strip(html[u'start tag'])) self.startTagLineEdit.setText(html[u'start html']) @@ -85,21 +93,26 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): self.tagLineEdit.setEnabled(False) self.startTagLineEdit.setEnabled(False) self.endTagLineEdit.setEnabled(False) - self.savePushButton.setEnabled(False) self.deletePushButton.setEnabled(False) else: self.descriptionLineEdit.setEnabled(True) self.tagLineEdit.setEnabled(True) self.startTagLineEdit.setEnabled(True) self.endTagLineEdit.setEnabled(True) - self.savePushButton.setEnabled(True) self.deletePushButton.setEnabled(True) + def onTextEdited(self, text): + """ + Enable the ``savePushButton`` when any of the selected tag's properties + has been changed. + """ + self.savePushButton.setEnabled(True) + def onNewClicked(self): """ Add a new tag to list only if it is not a duplicate. """ - for html in FormattingTags.html_expands: + for html in FormattingTags.get_html_tags(): if self._strip(html[u'start tag']) == u'n': critical_error_message_box( translate('OpenLP.FormattingTagForm', 'Update Error'), @@ -117,11 +130,13 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): u'temporary': False } FormattingTags.add_html_tags([tag]) - self._resetTable() + FormattingTags.save_html_tags() + self._reloadTable() # Highlight new row self.tagTableWidget.selectRow(self.tagTableWidget.rowCount() - 1) self.onRowSelected() self.tagTableWidget.scrollToBottom() + #self.savePushButton.setEnabled(False) def onDeleteClicked(self): """ @@ -130,14 +145,14 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): if self.selected != -1: FormattingTags.remove_html_tag(self.selected) self.selected = -1 - self._resetTable() - FormattingTags.save_html_tags() + FormattingTags.save_html_tags() + self._reloadTable() def onSavedClicked(self): """ Update Custom Tag details if not duplicate and save the data. """ - html_expands = FormattingTags.html_expands + html_expands = FormattingTags.get_html_tags() if self.selected != -1: html = html_expands[self.selected] tag = unicode(self.tagLineEdit.text()) @@ -157,14 +172,13 @@ class FormattingTagForm(QtGui.QDialog, Ui_FormattingTagDialog): # Keep temporary tags when the user changes one. html[u'temporary'] = False self.selected = -1 - self._resetTable() FormattingTags.save_html_tags() + self._reloadTable() - def _resetTable(self): + def _reloadTable(self): """ Reset List for loading. """ - FormattingTags.load_tags() self.tagTableWidget.clearContents() self.tagTableWidget.setRowCount(0) self.newPushButton.setEnabled(True) diff --git a/openlp/plugins/songs/lib/xml.py b/openlp/plugins/songs/lib/xml.py index fdcb1dd60..f30dc0f24 100644 --- a/openlp/plugins/songs/lib/xml.py +++ b/openlp/plugins/songs/lib/xml.py @@ -317,9 +317,7 @@ class OpenLyrics(object): tags_element = None match = re.search(u'\{/?\w+\}', song.lyrics, re.UNICODE) if match: - # Reset available tags. - FormattingTags.reset_html_tags() - # Named 'formatting' - 'format' is built-in fuction in Python. + # Named 'format_' - 'format' is built-in fuction in Python. format_ = etree.SubElement(song_xml, u'format') tags_element = etree.SubElement(format_, u'tags') tags_element.set(u'application', u'OpenLP') @@ -572,7 +570,8 @@ class OpenLyrics(object): for tag in FormattingTags.get_html_tags()] new_tags = [tag for tag in found_tags if tag[u'start tag'] not in existing_tag_ids] - FormattingTags.add_html_tags(new_tags, True) + FormattingTags.add_html_tags(new_tags) + FormattingTags.save_html_tags() def _process_lines_mixed_content(self, element, newlines=True): """ From 8b08fbbe3230ec1b77c4b142e73bc9a16152ac31 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 5 May 2012 20:34:26 +0200 Subject: [PATCH 19/19] docs and class variable fix --- openlp/core/lib/imagemanager.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index 10cfce616..b32e36194 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -100,7 +100,7 @@ class Image(object): variables ``image`` and ``image_bytes`` to ``None`` and add the image object to the queue of images to process. """ - NUMBER = 0 + secondary_priority = 0 def __init__(self, name, path, source, background): self.name = name self.path = path @@ -109,19 +109,20 @@ class Image(object): self.priority = Priority.Normal self.source = source self.background = background - self._number = Image.NUMBER - Image.NUMBER += 1 + self.secondary_priority = Image.secondary_priority + Image.secondary_priority += 1 class PriorityQueue(Queue.PriorityQueue): """ Customised ``Queue.PriorityQueue``. - Each item in the queue must be tuple with three values. The fist value - is the priority, the second value the image's ``_number`` attribute. The - last value the :class:`Image` instance itself:: + Each item in the queue must be tuple with three values. The first value + is the :class:`Image`'s ``priority`` attribute, the second value + the :class:`Image`'s ``secondary_priority`` attribute. The last value the + :class:`Image` instance itself:: - (Priority.Normal, image._number, image) + (image.priority, image.secondary_priority, image) Doing this, the :class:`Queue.PriorityQueue` will sort the images according to their priorities, but also according to there number. However, the number @@ -141,7 +142,7 @@ class PriorityQueue(Queue.PriorityQueue): """ self.remove(image) image.priority = new_priority - self.put((image.priority, image._number, image)) + self.put((image.priority, image.secondary_priority, image)) def remove(self, image): """ @@ -150,8 +151,8 @@ class PriorityQueue(Queue.PriorityQueue): ``image`` The image to remove. This should be an ``Image`` instance. """ - if (image.priority, image._number, image) in self.queue: - self.queue.remove((image.priority, image._number, image)) + if (image.priority, image.secondary_priority, image) in self.queue: + self.queue.remove((image.priority, image.secondary_priority, image)) class ImageManager(QtCore.QObject): @@ -276,7 +277,8 @@ class ImageManager(QtCore.QObject): if not name in self._cache: image = Image(name, path, source, background) self._cache[name] = image - self._conversion_queue.put((image.priority, image._number, image)) + self._conversion_queue.put( + (image.priority, image.secondary_priority, image)) else: log.debug(u'Image in cache %s:%s' % (name, path)) # We want only one thread.