Re: [Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
What is the problem with tests? We need to be able to test theses things Probably the main problem is, that I never worked with such test frameworks and therefore I do not know how to get access to the relevant functions. So I have to train a lot... ;) -- https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/179610 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/1194610_2.0 into lp:openlp/2.0
Are you going to resubmit this merge proposal? -- https://code.launchpad.net/~phill-ridout/openlp/1194610_2.0/+merge/17 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/1209515_2.0 into lp:openlp/2.0
Review: Needs Fixing Line 140 should be in there. PEP8 says that there should be 2 blank lines between statements on the module level. Oh, and even if it says you need to review yourself, please don't. -- https://code.launchpad.net/~phill-ridout/openlp/1209515_2.0/+merge/180471 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/1211913_2.0 into lp:openlp/2.0
Review: Approve -- https://code.launchpad.net/~phill-ridout/openlp/1211913_2.0/+merge/180234 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~phill-ridout/openlp/1096450_2.0 into lp:openlp/2.0
The proposal to merge lp:~phill-ridout/openlp/1096450_2.0 into lp:openlp/2.0 has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~phill-ridout/openlp/1096450_2.0/+merge/178455 -- https://code.launchpad.net/~phill-ridout/openlp/1096450_2.0/+merge/178455 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/1211913_2.0 into lp:openlp/2.0
Review: Needs Information Should the strings be u' -- https://code.launchpad.net/~phill-ridout/openlp/1211913_2.0/+merge/180234 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~sam92/openlp/openlp into lp:openlp
Please also note that anything being merged into lp:openlp is supposed to have a test associated with it. The test does not have to be related to the change, we just want to slowly work up our test coverage. -- https://code.launchpad.net/~sam92/openlp/openlp/+merge/178453 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~sam92/openlp/openlp into lp:openlp
Don't worry about this one, I'm going to merge it now. Just keep it in mind in future. -- https://code.launchpad.net/~sam92/openlp/openlp/+merge/178453 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~sam92/openlp/openlp into lp:openlp
The proposal to merge lp:~sam92/openlp/openlp into lp:openlp has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~sam92/openlp/openlp/+merge/178453 -- https://code.launchpad.net/~sam92/openlp/openlp/+merge/178453 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Oliver Wieland has proposed merging lp:~oliwee/openlp/HideBibleVerses into
lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Raoul Snyman (raoul-snyman)
Related bugs:
Bug #1051699 in OpenLP: Bibles add option to not display chapter and verse
numbers
https://bugs.launchpad.net/openlp/+bug/1051699
For more details, see:
https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651
The changes add a Checkbox at the Bibles settings tab to show or hide the verse
numbers. Therefor a new element Display verse numbers in the settings is
implemented. On default, this element is set to True.
If the value is set to False, the values Show new chapters and Display
Style are not relevant and the input fields at the Bibles settings tab will be
greyed out.
--
https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/bibles/bibleplugin.py'
--- openlp/plugins/bibles/bibleplugin.py 2013-04-18 17:45:14 +
+++ openlp/plugins/bibles/bibleplugin.py 2013-08-16 20:44:21 +
@@ -48,6 +48,7 @@
u'bibles/verse layout style': LayoutStyle.VersePerSlide,
u'bibles/book name language': LanguageSelection.Bible,
u'bibles/display brackets': DisplayStyle.NoBrackets,
+u'bibles/verse number display': True,
u'bibles/display new chapter': False,
u'bibles/second bibles': True,
u'bibles/advanced bible': u'',
=== modified file 'openlp/plugins/bibles/lib/biblestab.py'
--- openlp/plugins/bibles/lib/biblestab.py 2013-03-25 07:27:54 +
+++ openlp/plugins/bibles/lib/biblestab.py 2013-08-16 20:44:21 +
@@ -58,6 +58,9 @@
self.verse_display_group_box.setObjectName(u'verse_display_group_box')
self.verse_display_layout = QtGui.QFormLayout(self.verse_display_group_box)
self.verse_display_layout.setObjectName(u'verse_display_layout')
+self.verse_number_display_check_box = QtGui.QCheckBox(self.verse_display_group_box)
+self.verse_number_display_check_box.setObjectName(u'verse_display_check_box')
+self.verse_display_layout.addRow(self.verse_number_display_check_box)
self.new_chapters_check_box = QtGui.QCheckBox(self.verse_display_group_box)
self.new_chapters_check_box.setObjectName(u'new_chapters_check_box')
self.verse_display_layout.addRow(self.new_chapters_check_box)
@@ -134,6 +137,7 @@
self.left_layout.addStretch()
self.right_layout.addStretch()
# Signals and slots
+self.verse_number_display_check_box.stateChanged.connect(self.on_verse_number_display_check_box_changed)
self.new_chapters_check_box.stateChanged.connect(self.on_new_chapters_check_box_changed)
self.display_style_combo_box.activated.connect(self.on_display_style_combo_box_changed)
self.bible_theme_combo_box.activated.connect(self.on_bible_theme_combo_box_changed)
@@ -156,6 +160,7 @@
def retranslateUi(self):
self.verse_display_group_box.setTitle(translate('BiblesPlugin.BiblesTab', 'Verse Display'))
+self.verse_number_display_check_box.setText(translate('BiblesPlugin.BiblesTab', 'Display verse numbers'))
self.new_chapters_check_box.setText(translate('BiblesPlugin.BiblesTab', 'Only show new chapter numbers'))
self.layout_style_label.setText(UiStrings().LayoutStyle)
self.display_style_label.setText(UiStrings().DisplayStyle)
@@ -208,6 +213,13 @@
def on_language_selection_combo_box_changed(self):
self.language_selection = self.language_selection_combo_box.currentIndex()
+def on_verse_number_display_check_box_changed(self, check_state):
+self.verse_number_display = False
+# We have a set value convert to True/False.
+if check_state == QtCore.Qt.Checked:
+self.verse_number_display = True
+self.check_verse_number_display()
+
def on_new_chapters_check_box_changed(self, check_state):
self.show_new_chapters = False
# We have a set value convert to True/False.
@@ -299,11 +311,14 @@
def load(self):
settings = Settings()
settings.beginGroup(self.settings_section)
+self.verse_number_display = settings.value(u'verse number display')
self.show_new_chapters = settings.value(u'display new chapter')
self.display_style = settings.value(u'display brackets')
self.layout_style = settings.value(u'verse layout style')
self.bible_theme = settings.value(u'bible theme')
self.second_bibles = settings.value(u'second bibles')
+self.verse_number_display_check_box.setChecked(self.verse_number_display)
+self.check_verse_number_display()
self.new_chapters_check_box.setChecked(self.show_new_chapters)
self.display_style_combo_box.setCurrentIndex(self.display_style)
self.layout_style_combo_box.setCurrentIndex(self.layout_style)
@@ -351,6 +366,7 @@
def save(self):
[Openlp-core] [Bug 1096450] Re: Opensong Import - some lines missing
** Branch linked: lp:openlp/2.0 -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1096450 Title: Opensong Import - some lines missing Status in OpenLP - Worship Presentation Software: In Progress Status in OpenLP 2.0 series: Fix Committed Status in OpenLP trunk series: In Progress Bug description: When the following verse is imported from following Opensong format lyrics[V1] 1. Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. [V2] It create the following OpenLP format: ---[Vers:1]--- . Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. [---] . Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. ---[Vers:2]--- This happen on Version 1.9.12 and 2.0 of OpenLP and both for Windows and Mac. There other cases when some lines are missing or not imported properly: - line starting with '[' - this line is missing in the lyrics - line starting with '.' - these lines indicate chords and it should be ignored when importing into openlp. To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1096450/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Bug 1096450] Re: Opensong Import - some lines missing
** Changed in: openlp/2.0 Status: In Progress = Fix Committed -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1096450 Title: Opensong Import - some lines missing Status in OpenLP - Worship Presentation Software: In Progress Status in OpenLP 2.0 series: Fix Committed Status in OpenLP trunk series: In Progress Bug description: When the following verse is imported from following Opensong format lyrics[V1] 1. Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. [V2] It create the following OpenLP format: ---[Vers:1]--- . Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. [---] . Gud vår Gud, vi lovar dig, vi bekänner dig, o Herre, dig som är till evig tid tillber ändligheternas släkten. Hela jorden, hav och land, ärar, fader vår, ditt namn. ---[Vers:2]--- This happen on Version 1.9.12 and 2.0 of OpenLP and both for Windows and Mac. There other cases when some lines are missing or not imported properly: - line starting with '[' - this line is missing in the lyrics - line starting with '.' - these lines indicate chords and it should be ignored when importing into openlp. To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1096450/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~sam92/openlp/bug-1094809 into lp:openlp
The proposal to merge lp:~sam92/openlp/bug-1094809 into lp:openlp has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~sam92/openlp/bug-1094809/+merge/178452 -- https://code.launchpad.net/~sam92/openlp/bug-1094809/+merge/178452 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Review: Needs Fixing Line 30 206 looks strange # WHEN etc. Need a space -- https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Bug 1094809] Re: Song Editor - improve usability of Verse Order field
** Branch linked: lp:openlp -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1094809 Title: Song Editor - improve usability of Verse Order field Status in OpenLP - Worship Presentation Software: Fix Committed Bug description: Today I was attempting to use the Verse order field in the Song Editor. I was manually creating a new song (copy/paste lyrics from internet site into individual verses) and I put them in the wrong order in the Song Editor (put verse 4 ahead of verse 3). Since I'm new to OpenLP I decided to play around with things, so I attempted to use the Verse order field to order the verses correctly. I tried putting in V1, V2, C1, V4, V3 and it popped up an error message saying the order was invalid. Then I tried multiple other combinations until I found out that the delimiter had to be a space. The error message states something along the lines of The verse order is invalid. There are no verses corresponding to V1,, V2,, C1,, V4,, V3. Valid entries are V1, V2, C1, V4, and V3. 1) Add some onscreen help text/validation to prevent users from entering information in an invalid format. Potentially shorten the Lyrics field by a few rows to allow more space for this. 2) The error message as currently written doesn't actually tell users how to fix the problem. An error message that says something like Add each entry abbreviation (V1, V2, etc) separated by a single space would guide users as to how to fix the problem more quickly. OS: Win 7 home premium LP Version: 2.0 To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1094809/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Bug 1094809] Re: Song Editor - improve usability of Verse Order field
** Changed in: openlp Status: New = Fix Committed -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1094809 Title: Song Editor - improve usability of Verse Order field Status in OpenLP - Worship Presentation Software: Fix Committed Bug description: Today I was attempting to use the Verse order field in the Song Editor. I was manually creating a new song (copy/paste lyrics from internet site into individual verses) and I put them in the wrong order in the Song Editor (put verse 4 ahead of verse 3). Since I'm new to OpenLP I decided to play around with things, so I attempted to use the Verse order field to order the verses correctly. I tried putting in V1, V2, C1, V4, V3 and it popped up an error message saying the order was invalid. Then I tried multiple other combinations until I found out that the delimiter had to be a space. The error message states something along the lines of The verse order is invalid. There are no verses corresponding to V1,, V2,, C1,, V4,, V3. Valid entries are V1, V2, C1, V4, and V3. 1) Add some onscreen help text/validation to prevent users from entering information in an invalid format. Potentially shorten the Lyrics field by a few rows to allow more space for this. 2) The error message as currently written doesn't actually tell users how to fix the problem. An error message that says something like Add each entry abbreviation (V1, V2, etc) separated by a single space would guide users as to how to fix the problem more quickly. OS: Win 7 home premium LP Version: 2.0 To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1094809/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Line 30 206 looks strange Do you mean the indentation in 206? What's wrong in line 30? -- https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Review: Needs Fixing Hi Oli, I'm just looking at your recent rename, and it doesn't quite make sense to me. If the variable is called verse_number_display it makes me think this tells me how the verse number is to be displayed. However, since the variable is attached to a checkbox, I realised that it is actually whether the verse number should be hidden or visible. Might I suggest renaming the variable to: is_verse_number_visible self.is_verse_number_visible = True # You can see the verse numbers self.is_verse_number_visible = False # Now you can't see them Then you should rename the checkbox too: self.is_verse_number_visible_check_box = QtGui.QCheckBox() Now, on to the tests. Firstly, I want to say that I am very impressed with your first attempt at a test. This looks really good, and it looks like you're catching on to how to write tests pretty quickly. Keep on! Just a few minor details (mostly style): - Line 138 does not need to be blank. You can remove it. - Please write a docstring for the TestVerseReferenceList class. - Line 146: add_test(self) is not a well named method. Rather name it add_verse_reference_test - Line 150 does not need to be blank. You can remove it. - You should never have more than 1 GIVEN, WHEN, THEN set in your test. If you have more than one, break your test up into more tests. You can start the next test with a GIVEN: 1 line in the list of verses. - There should be a space between the # and GIVEN/WHEN/THEN e.g. # GIVEN: An empty verse list - There should be spaces either side of a %, e.g. %s % book - If you're adding items to a list of some sort, I highly recommend checking len(list) - Once again, extra blank lines are not necessary. Please remove lines 200, 208 and 218. - Even in tests, variable names should be PEP8 compliant: old_len, not oldLen. - Don't call __len__(), use len(self.reference_list.version_list) -- https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Bug 1094809] Re: Song Editor - improve usability of Verse Order field
** Also affects: openlp/trunk Importance: Wishlist Status: Fix Committed ** Changed in: openlp/trunk Milestone: None = 2.0.2 ** Changed in: openlp/trunk Milestone: 2.0.2 = 2.1.1 ** Changed in: openlp/trunk Assignee: (unassigned) = Samuel Mehrbrodt (sam92) -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1094809 Title: Song Editor - improve usability of Verse Order field Status in OpenLP - Worship Presentation Software: Fix Committed Status in OpenLP trunk series: Fix Committed Bug description: Today I was attempting to use the Verse order field in the Song Editor. I was manually creating a new song (copy/paste lyrics from internet site into individual verses) and I put them in the wrong order in the Song Editor (put verse 4 ahead of verse 3). Since I'm new to OpenLP I decided to play around with things, so I attempted to use the Verse order field to order the verses correctly. I tried putting in V1, V2, C1, V4, V3 and it popped up an error message saying the order was invalid. Then I tried multiple other combinations until I found out that the delimiter had to be a space. The error message states something along the lines of The verse order is invalid. There are no verses corresponding to V1,, V2,, C1,, V4,, V3. Valid entries are V1, V2, C1, V4, and V3. 1) Add some onscreen help text/validation to prevent users from entering information in an invalid format. Potentially shorten the Lyrics field by a few rows to allow more space for this. 2) The error message as currently written doesn't actually tell users how to fix the problem. An error message that says something like Add each entry abbreviation (V1, V2, etc) separated by a single space would guide users as to how to fix the problem more quickly. OS: Win 7 home premium LP Version: 2.0 To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1094809/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Bug 1213254] [NEW] OpenLP 1 database importer crashes when encountering control chars
Public bug reported: http://support.openlp.org/issues/1941 --- Exception Traceback --- Traceback (most recent call last): File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.core.ui.wizard, line 188, in onCurrentIdChanged File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.forms.songimportform, line 391, in performWizard File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.olp1import, line 161, in doImport File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.songimport, line 327, in finish File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.xml, line 119, in add_verse_to_lyrics File lxml.etree.pyx, line 2613, in lxml.etree.CDATA.__init__ (src/lxml/lxml.etree.c:51419) File apihelpers.pxi, line 1366, in lxml.etree._utf8 (src/lxml/lxml.etree.c:22211) ValueError: All strings must be XML compatible: Unicode or ASCII, no NULL bytes or control characters ** Affects: openlp Importance: Undecided Status: New ** Affects: openlp/2.0 Importance: Undecided Status: Confirmed ** Tags: song-import support-system ** Also affects: openlp/2.0 Importance: Undecided Status: New ** Changed in: openlp/2.0 Status: New = Confirmed ** Changed in: openlp/2.0 Milestone: None = 2.0.2 -- You received this bug notification because you are a member of OpenLP Core, which is subscribed to OpenLP. https://bugs.launchpad.net/bugs/1213254 Title: OpenLP 1 database importer crashes when encountering control chars Status in OpenLP - Worship Presentation Software: New Status in OpenLP 2.0 series: Confirmed Bug description: http://support.openlp.org/issues/1941 --- Exception Traceback --- Traceback (most recent call last): File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.core.ui.wizard, line 188, in onCurrentIdChanged File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.forms.songimportform, line 391, in performWizard File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.olp1import, line 161, in doImport File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.songimport, line 327, in finish File D:\OpenLP_Code\release-2.0.1\build\pyi.win32\OpenLP\out00-PYZ.pyz\openlp.plugins.songs.lib.xml, line 119, in add_verse_to_lyrics File lxml.etree.pyx, line 2613, in lxml.etree.CDATA.__init__ (src/lxml/lxml.etree.c:51419) File apihelpers.pxi, line 1366, in lxml.etree._utf8 (src/lxml/lxml.etree.c:22211) ValueError: All strings must be XML compatible: Unicode or ASCII, no NULL bytes or control characters To manage notifications about this bug go to: https://bugs.launchpad.net/openlp/+bug/1213254/+subscriptions ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
Re: [Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Hi Raoul, thanks for the review. I'm glad to hear that I haven't coded total nonsense ;) You should never have more than 1 GIVEN, WHEN, THEN set in your test. If you have more than one, break your test up into more tests. You can start the next test with a GIVEN: 1 line in the list of verses. I tried that before, but the tests depend on each other because of the count of entries in the list. If I separate them, it seems that they will not be executed in the given order, so they will fail. Maybe I should create the list locally and fill it to the GIVEN state, then there should be no dependencies between separate tests. You are right with the variable names. Choosing appropriate variable names has never been my strong point. I will change it like you suggested. -- https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
[Openlp-core] [Merge] lp:~phill-ridout/openlp/1194610_2.0 into lp:openlp/2.0
Phill has proposed merging lp:~phill-ridout/openlp/1194610_2.0 into lp:openlp/2.0. Requested reviews: Tim Bentley (trb143) Raoul Snyman (raoul-snyman) Related bugs: Bug #1194610 in OpenLP: [support-system] SongShowPlus traceback https://bugs.launchpad.net/openlp/+bug/1194610 For more details, see: https://code.launchpad.net/~phill-ridout/openlp/1194610_2.0/+merge/180662 Fixes bug 1194610 by detecting the encoding rather than assuming that it set (because its not) add fall back to the windows encoding dialog -- https://code.launchpad.net/~phill-ridout/openlp/1194610_2.0/+merge/180662 Your team OpenLP Core is subscribed to branch lp:openlp/2.0. === modified file 'openlp/plugins/songs/lib/songshowplusimport.py' --- openlp/plugins/songs/lib/songshowplusimport.py 2013-02-18 17:15:44 + +++ openlp/plugins/songs/lib/songshowplusimport.py 2013-08-16 22:09:13 + @@ -30,13 +30,14 @@ The :mod:`songshowplusimport` module provides the functionality for importing SongShow Plus songs into the OpenLP database. +import chardet import os import logging import re import struct from openlp.core.ui.wizard import WizardStrings -from openlp.plugins.songs.lib import VerseType +from openlp.plugins.songs.lib import VerseType, retrieve_windows_encoding from openlp.plugins.songs.lib.songimport import SongImport TITLE = 1 @@ -142,44 +143,44 @@ log.debug(length_descriptor_size) data = song_data.read(length_descriptor) if block_key == TITLE: -self.title = unicode(data, u'cp1252') +self.title = self.decode(data) elif block_key == AUTHOR: authors = data.split( / ) for author in authors: if author.find(,) !=-1: authorParts = author.split(, ) author = authorParts[1] + + authorParts[0] -self.parseAuthor(unicode(author, u'cp1252')) +self.parseAuthor(self.decode(author)) elif block_key == COPYRIGHT: -self.addCopyright(unicode(data, u'cp1252')) +self.addCopyright(self.decode(data)) elif block_key == CCLI_NO: self.ccliNumber = int(data) elif block_key == VERSE: -self.addVerse(unicode(data, u'cp1252'), +self.addVerse(self.decode(data), %s%s % (VerseType.Tags[VerseType.Verse], verse_no)) elif block_key == CHORUS: -self.addVerse(unicode(data, u'cp1252'), +self.addVerse(self.decode(data), %s%s % (VerseType.Tags[VerseType.Chorus], verse_no)) elif block_key == BRIDGE: -self.addVerse(unicode(data, u'cp1252'), +self.addVerse(self.decode(data), %s%s % (VerseType.Tags[VerseType.Bridge], verse_no)) elif block_key == TOPIC: -self.topics.append(unicode(data, u'cp1252')) +self.topics.append(self.decode(data)) elif block_key == COMMENTS: -self.comments = unicode(data, u'cp1252') +self.comments = self.decode(data) elif block_key == VERSE_ORDER: verse_tag = self.toOpenLPVerseTag(data, True) if verse_tag: if not isinstance(verse_tag, unicode): -verse_tag = unicode(verse_tag, u'cp1252') +verse_tag = self.decode(verse_tag) self.sspVerseOrderList.append(verse_tag) elif block_key == SONG_BOOK: -self.songBookName = unicode(data, u'cp1252') +self.songBookName = self.decode(data) elif block_key == SONG_NUMBER: self.songNumber = ord(data) elif block_key == CUSTOM_VERSE: verse_tag = self.toOpenLPVerseTag(verse_name) -self.addVerse(unicode(data, u'cp1252'), verse_tag) +self.addVerse(self.decode(data), verse_tag) else: log.debug(Unrecognised blockKey: %s, data: %s % (block_key, data)) @@ -221,3 +222,9 @@ verse_tag = VerseType.Tags[VerseType.Other] verse_number = self.otherList[verse_name] return verse_tag + verse_number + +def decode(self, data): +try: +return unicode(data, chardet.detect(data)['encoding']) +except: +return unicode(data, u'cp1252') \ No newline at end of file ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe :
[Openlp-core] [Merge] lp:~oliwee/openlp/HideBibleVerses into lp:openlp
Oliver Wieland has proposed merging lp:~oliwee/openlp/HideBibleVerses into
lp:openlp.
Requested reviews:
Raoul Snyman (raoul-snyman)
Tim Bentley (trb143)
Related bugs:
Bug #1051699 in OpenLP: Bibles add option to not display chapter and verse
numbers
https://bugs.launchpad.net/openlp/+bug/1051699
For more details, see:
https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180663
The changes add a Checkbox at the Bibles settings tab to show or hide the verse
numbers. Therefor a new element Display verse numbers in the settings is
implemented. On default, this element is set to True.
If the value is set to False, the values Show new chapters and Display
Style are not relevant and the input fields at the Bibles settings tab will be
greyed out.
--
https://code.launchpad.net/~oliwee/openlp/HideBibleVerses/+merge/180663
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/bibles/bibleplugin.py'
--- openlp/plugins/bibles/bibleplugin.py 2013-04-18 17:45:14 +
+++ openlp/plugins/bibles/bibleplugin.py 2013-08-16 22:16:45 +
@@ -48,6 +48,7 @@
u'bibles/verse layout style': LayoutStyle.VersePerSlide,
u'bibles/book name language': LanguageSelection.Bible,
u'bibles/display brackets': DisplayStyle.NoBrackets,
+u'bibles/is verse number visible': True,
u'bibles/display new chapter': False,
u'bibles/second bibles': True,
u'bibles/advanced bible': u'',
=== modified file 'openlp/plugins/bibles/lib/biblestab.py'
--- openlp/plugins/bibles/lib/biblestab.py 2013-03-25 07:27:54 +
+++ openlp/plugins/bibles/lib/biblestab.py 2013-08-16 22:16:45 +
@@ -58,6 +58,9 @@
self.verse_display_group_box.setObjectName(u'verse_display_group_box')
self.verse_display_layout = QtGui.QFormLayout(self.verse_display_group_box)
self.verse_display_layout.setObjectName(u'verse_display_layout')
+self.is_verse_number_visible_check_box = QtGui.QCheckBox(self.verse_display_group_box)
+self.is_verse_number_visible_check_box.setObjectName(u'verse_display_check_box')
+self.verse_display_layout.addRow(self.is_verse_number_visible_check_box)
self.new_chapters_check_box = QtGui.QCheckBox(self.verse_display_group_box)
self.new_chapters_check_box.setObjectName(u'new_chapters_check_box')
self.verse_display_layout.addRow(self.new_chapters_check_box)
@@ -134,6 +137,7 @@
self.left_layout.addStretch()
self.right_layout.addStretch()
# Signals and slots
+self.is_verse_number_visible_check_box.stateChanged.connect(self.on_is_verse_number_visible_check_box_changed)
self.new_chapters_check_box.stateChanged.connect(self.on_new_chapters_check_box_changed)
self.display_style_combo_box.activated.connect(self.on_display_style_combo_box_changed)
self.bible_theme_combo_box.activated.connect(self.on_bible_theme_combo_box_changed)
@@ -156,6 +160,7 @@
def retranslateUi(self):
self.verse_display_group_box.setTitle(translate('BiblesPlugin.BiblesTab', 'Verse Display'))
+self.is_verse_number_visible_check_box.setText(translate('BiblesPlugin.BiblesTab', 'Is verse number visible'))
self.new_chapters_check_box.setText(translate('BiblesPlugin.BiblesTab', 'Only show new chapter numbers'))
self.layout_style_label.setText(UiStrings().LayoutStyle)
self.display_style_label.setText(UiStrings().DisplayStyle)
@@ -208,6 +213,13 @@
def on_language_selection_combo_box_changed(self):
self.language_selection = self.language_selection_combo_box.currentIndex()
+def on_is_verse_number_visible_check_box_changed(self, check_state):
+self.is_verse_number_visible = False
+# We have a set value convert to True/False.
+if check_state == QtCore.Qt.Checked:
+self.is_verse_number_visible = True
+self.check_is_verse_number_visible()
+
def on_new_chapters_check_box_changed(self, check_state):
self.show_new_chapters = False
# We have a set value convert to True/False.
@@ -299,11 +311,14 @@
def load(self):
settings = Settings()
settings.beginGroup(self.settings_section)
+self.is_verse_number_visible = settings.value(u'is verse number visible')
self.show_new_chapters = settings.value(u'display new chapter')
self.display_style = settings.value(u'display brackets')
self.layout_style = settings.value(u'verse layout style')
self.bible_theme = settings.value(u'bible theme')
self.second_bibles = settings.value(u'second bibles')
+self.is_verse_number_visible_check_box.setChecked(self.is_verse_number_visible)
+self.check_is_verse_number_visible()
self.new_chapters_check_box.setChecked(self.show_new_chapters)
self.display_style_combo_box.setCurrentIndex(self.display_style)
