[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-25 Thread noreply
The proposal to merge lp:~phill-ridout/openlp/bug-1011286 into lp:openlp has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/119419 -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/119419

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-24 Thread Tim Bentley
Review: Approve -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/119419 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-19 Thread phill
Any chance of getting this in before the next freeze? -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/119419 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-19 Thread Raoul Snyman
Review: Approve -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/119419 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-13 Thread Andreas Preikschat
Review: Needs Information I am not sure about line 9. I do not see a place where previous_raw becomes None. My question: Why did you change the line? In what circumstances does this fix or prevent something? Currently I believe that this check is not needed any longer (because it is always

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-13 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Tim Bentley (trb143) Jonathan Corwin (j-corwin) Andreas Preikschat (googol) Raoul Snyman (raoul-snyman) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-06 Thread Raoul Snyman
Review: Approve I am happy with this, let's get it in. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-08-06 Thread Tim Bentley
Review: Approve looks fine to me to. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to :

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-27 Thread Tim Bentley
I have not had time this week to review this due to pressure. It is too late to get this in and tested before the freeze. This needs to be reviewed and signed off by two Core's ! -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 Your team OpenLP Core is subscribed to

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-27 Thread phill
I have not had time this week to review this due to pressure. It is too late to get this in and tested before the freeze. This needs to be reviewed and signed off by two Core's ! Thats fine, although I've done my testing, I would rather it was thourghly tested. Due to the change to the render

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-27 Thread googol
Due to the change to the render it has the potential to screw the main display up. By the way, what is the change of line 8/9 meant to do? Why did you change it? ___ Mailing list: https://launchpad.net/~openlp-core Post to :

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-27 Thread Raoul Snyman
Yes, let's play it safe. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to :

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-26 Thread Raoul Snyman
I just want to check, is this proposal waiting for post-RC2? -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-26 Thread phill
No, not if you are happy with it. On Jul 27, 2012 12:10 AM, Raoul Snyman raoul.sny...@saturnlaboratories.co.za wrote: I just want to check, is this proposal waiting for post-RC2? -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/116168 You are the owner of

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-22 Thread Tim Bentley
Review: Approve All verses must have 1 character in them The last verse must have 2 characters in it to be preserved. This is strange behaviour but if this is an existing bug then that needs to be looked at under a new bug. --

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Raoul Snyman (raoul-snyman) Jonathan Corwin (j-corwin) Tim Bentley (trb143) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-22 Thread phill
All verses must have 1 character in them The last verse must have 2 characters in it to be preserved. This is strange behaviour but if this is an existing bug then that needs to be looked at under a new bug. Both these issues have now been fixed in my latest resubmit --

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Raoul Snyman (raoul-snyman) Jonathan Corwin (j-corwin) Tim Bentley (trb143) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-21 Thread Tim Bentley
Review: Needs Fixing Sorry but the saving has become a mess. Add a song ---[Verse:1]--- ---[Verse:2]--- does not allow the song to be saved. ---[Verse:1]--- ---[Verse:2]--- with a space in each verse allows the song to be saved but verse 2 disappears. The loss of verse 2 even with a space in

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-21 Thread Jonathan Corwin
Tim, I just tried your second example, and verse 2 doesn't disappear for me. I don't see how Phill's changes could affect this behaviour. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/114992 Your team OpenLP Core is subscribed to branch lp:openlp.

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-21 Thread phill
Hrm, It removes the last verse if there is only one char after the new line. It can be any char it appears! Although, technically this has nothing to do with my code, my code is doing what it should do. This is caused by the code that creates the list items. I had a look at it, but its too far

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-14 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Raoul Snyman (raoul-snyman) Tim Bentley (trb143) Jonathan Corwin (j-corwin) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-12 Thread Jonathan Corwin
Review: Needs Fixing I think line 21 should now be the following to replicate previous behaviour? if not value[1]: -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/114507 Your team OpenLP Core is subscribed to branch lp:openlp.

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-11 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Jonathan Corwin (j-corwin) Raoul Snyman (raoul-snyman) Tim Bentley (trb143) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-07-05 Thread Tim Bentley
Review: Resubmit -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/110651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Raoul Snyman (raoul-snyman) Tim Bentley (trb143) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Tim Bentley
Review: Needs Fixing Looks OK but we are in a string freeze so this cannot be merge. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/110651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list:

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread phill
On Jun 16, 2012 9:42 AM, Tim Bentley tim.bent...@gmail.com wrote: Review: Needs Fixing Looks OK but we are in a string freeze so this cannot be merge. I have not changed any strings. One has moved up a couple of lines but I thought that was neither here nor there. --

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Tim Bentley
Review: Approve Needs to put his glasses on! -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/110651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to :

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Jonathan Corwin
Review: Needs Fixing If I now Edit All, remove all the text from just one of the verses: ---[Verse:1]--- ---[Verse:2]--- blah blah blah I now don't get the error dialog, but verse 1 also completely disappears when I save it. We still need that warning for the edit all scenario. We just don't

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread phill
On 16 June 2012 13:59, Jonathan Corwin j...@corwin.co.uk wrote: I now don't get the error dialog, but verse 1 also completely disappears when I save it. We still need that warning for the edit all scenario. We just don't want it to crash if all the text is removed. So its ok that V1

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Raoul Snyman
Review: Approve So, apparently what I want is a change in functionality. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/110651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list:

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Jonathan Corwin
The existing code was preventing empty verses. ---[Verse:1]--- ---[Verse:2]--- We still need the code that was preventing it. So V1 won't disappear because the dialog box will prevent the above from being saved. We don't need a dialog if there is no text at all in the edit all verses window.

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-16 Thread Jonathan Corwin
(Needs Fixing still applies) -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/110651 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to :

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-13 Thread phill
On Jun 12, 2012 8:06 PM, Raoul Snyman raoul.sny...@saturnlaboratories.co.za wrote: Review: Needs Fixing Can you clarify what you want? The current behaviour is: Click Add/Edit verse try to close the dialog box by clicking save, to save with out any text and there is a message box telling the

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-12 Thread Raoul Snyman
Review: Needs Fixing You should be able to close the dialog via OK without any text in the dialog. No text is also valid. -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/109568 Your team OpenLP Core is subscribed to branch lp:openlp.

[Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-11 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug-1011286 into lp:openlp. Requested reviews: Tim Bentley (trb143) Related bugs: Bug #1011286 in OpenLP: Song Editor - Edit All Crashes with out valid verse splitter https://bugs.launchpad.net/openlp/+bug/1011286 For more details, see:

Re: [Openlp-core] [Merge] lp:~phill-ridout/openlp/bug-1011286 into lp:openlp

2012-06-10 Thread Tim Bentley
Review: Needs Fixing Do we need the log.debug? -- https://code.launchpad.net/~phill-ridout/openlp/bug-1011286/+merge/109540 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to :