[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-06-01 Thread noreply
The proposal to merge lp:~gerald-britton/openlp/newbugs into lp:openlp has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 Your te

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-06-01 Thread Raoul Snyman
The proposal to merge lp:~gerald-britton/openlp/newbugs into lp:openlp has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 Y

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-06-01 Thread Raoul Snyman
Review: Approve -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubs

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-31 Thread Jonathan Corwin
Review: Approve -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/63035 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubs

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-31 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: Jonathan Corwin (j-corwin) Tim Bentley (trb143) Raoul Snyman (raoul-snyman) jerryb (gerald-britton) Andreas Preikschat (googol-hush) For more details, see: https://code.launchpad.net/~gerald

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-31 Thread jerryb
Review: Resubmit hmmm... You are correct in that the intention is to raise an exception if all attempts to connect with uno fail. As it stands, it will only allow one failure before raising an exception. There are two ways to remedy this: 1. add an undented "else:" before the raise in line 73,

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-31 Thread Jonathan Corwin
Review: Needs Information Line 73: Is this "raise" at the correct indent, I'm wondering if should be outside the while loop? Also if you commit new changes to your branch, even a merge with trunk, please re-submit the merge proposal. Thanks. -- https://code.launchpad.net/~gerald-britton/openlp

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-31 Thread m2j
Just checked. You're right. It seems to be a problem, which occurred in a previous version of Python. So it is OK now. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62532 Your team OpenLP Core is subscribed to branch lp:openlp. __

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-30 Thread Raoul Snyman
Review: Approve Looks OK to me. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62921 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launc

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-30 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: Tim Bentley (trb143) Andreas Preikschat (googol-hush) Jonathan Corwin (j-corwin) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62921 This patch adds so

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-30 Thread jerryb
Thanks! I'll make the mod... On Mon, May 30, 2011 at 5:05 PM, Jonathan Corwin wrote: > The problem is line 98 of the diff: +from com.sun.star.uno import > RuntimeException > > This needs to be in a if os.name != u'nt': block, as we don't have pyuno on > Windows so win32com is used instead. > >

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-30 Thread Jonathan Corwin
The problem is line 98 of the diff: +from com.sun.star.uno import RuntimeException This needs to be in a if os.name != u'nt': block, as we don't have pyuno on Windows so win32com is used instead. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62780 Your team OpenLP Core is

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread jerryb
Hmmm... I cannot test with Windows 7 so I'll need your help (or someone's). From what I can see after a quick look, the error message is produced before any code in my patch is executed. More precisely, in importer.py at line 46, I see: try: from sofimport import SofImport HAS_SOF = True

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread Jonathan Corwin
LibreOffice 3.3 on Windows 7. Start OpenLP. Go to File -> Import -> Song. Click Next. Select Songs of Fellowship. You see the message that Songs of Fellowship has been disabled. See the screen shot: http://imageshack.us/f/101/sofproblems.png/ First is of this branch SoF Second is this branch Ge

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread jerryb
Can you please post the steps to reproduce the error and send me a screenshot? i am unable to make it fail that way. -- Sent via Mobile Mail --Original Message-- From: Jonathan Corwin To: "jerryb" Date: Sunday, May 29, 2011 12:12:32 PM GMT- Subject: Re: [Merge] lp:~gerald-

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread Jonathan Corwin
Review: Needs Fixing I can import Songs of Fellowship in latest trunk fine. I cannot import Songs of Fellowship in your branch. I am not going to introduce a regression into trunk, regardless of whether it is due to a pre-existing condition or not. -- https://code.launchpad.net/~gerald-britton/o

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread jerryb
On Sat, May 28, 2011 at 5:58 PM, Jonathan Corwin wrote: > Review: Needs Fixing > Btw I also agree with the indenty stuff too, but can only review based on the > existing standards! true enough, though here, what "needs fixing" is the standard, not the patch. >But that's a discussion for anothe

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread jerryb
On Sun, May 29, 2011 at 6:38 AM, m2j wrote: > Hi, > In my understanding there is a problem in patch line 81: > log.exception("open_ooo_file failed: %s", url) > url is Unicode and such it may contain non-ASCII characters. In that case you > get a error, as it is not possible to convert it to a 8 b

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-29 Thread m2j
Hi, In my understanding there is a problem in patch line 81: log.exception("open_ooo_file failed: %s", url) url is Unicode and such it may contain non-ASCII characters. In that case you get a error, as it is not possible to convert it to a 8 bit string automatically. Rather write: log.exception(u

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread Jonathan Corwin
Review: Needs Fixing Btw I also agree with the indenty stuff too, but can only review based on the existing standards! But that's a discussion for another place. Back to the matter in hand... If I go to the Song Import, and select Songs of Fellowship, it tells me that I haven't got OpenOffice i

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread Raoul Snyman
Gerald, funnily enough, I agree with you. Bring the indentation standard up on the mailing list, and we'll thrash it out there. Let's leave merge proposals for merging code into trunk. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62780 Your team OpenLP Core is subscribed t

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: Andreas Preikschat (googol-hush) Tim Bentley (trb143) Jonathan Corwin (j-corwin) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62780 This patch adds so

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread jerryb
> Gerald, please can you just conform to our at times convoluted and weird > standards. It's better having weird ones that none at all. Raoul, one could say that it's better being mostly dead than all dead (think Miracle Max in The Princess Bride), but that's not saying much. In this context, w

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread Raoul Snyman
Gerald, please can you just conform to our at times convoluted and weird standards. It's better having weird ones that none at all. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62532 Your team OpenLP Core is subscribed to branch lp:openlp. _

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread jerryb
> Indent wrong on 45, 46 & 49, 50 No, the indent is correct there. One should always indent on a continued line within an expression relative to the expression. To do otherwise gives the misleading appearance that the continued material belongs with the expression's surrounding environment in

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-28 Thread Jonathan Corwin
Review: Needs Fixing Indent wrong on 45, 46 & 49, 50 -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62532 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : op

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: Andreas Preikschat (googol-hush) Tim Bentley (trb143) Jonathan Corwin (j-corwin) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62532 This patch adds so

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread Andreas Preikschat
> 131 needs some context text. That is correct as it is now. > Well, actually we don't know that it doesn't exist. I must have misread that. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62484 Your team OpenLP Core is subscribed to branch lp:openlp.

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread jerryb
On Thu, May 26, 2011 at 12:42 PM, Tim Bentley wrote: > Review: Needs Fixing > translate does not work properly with u'' strings so they need to be removed. > > 131 needs some context text. I just followed other importers for that line. e.g. openlp/plugins/songs/lib/cclifileimport.py:84: self.log

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread Tim Bentley
Review: Needs Fixing translate does not work properly with u'' strings so they need to be removed. 131 needs some context text. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62484 Your team OpenLP Core is subscribed to branch lp:openlp. _

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread Jonathan Corwin
> >(Btw, we don't use unicode strings for strings in translate). > > OK -- but does it cause a problem? Do you want the unicode removed? > Yes, it causes a problem, it stops the string extraction tool from working. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62484 Yo

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread jerryb
On Thu, May 26, 2011 at 11:45 AM, Andreas Preikschat wrote: > Review: Needs Fixing > Hello, > > Please remove the lambda. It is rather confusing than helpful (the call has > to be made anyway). That's a rather subjective statement that I can't agree with. OTOH if we need multiple error messages

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread Andreas Preikschat
Review: Needs Fixing Hello, Please remove the lambda. It is rather confusing than helpful (the call has to be made anyway). And 'Unable to open file' is not the right error message when the file does not exist. (Btw, we don't use unicode strings for strings in translate). Sorry... -- https:/

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: Andreas Preikschat (googol-hush) Jonathan Corwin (j-corwin) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62484 This patch adds some robustness to the Op

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread jerryb
On Thu, May 26, 2011 at 3:41 AM, Andreas Preikschat wrote: > Review: Needs Fixing > Hello Gerald, > > You need to provide error messages in lines 44 and 46 (if you do not provide > one, the default one is taken, which says "Title and/or verses not found"). Yup, though I thought was OK. However,

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-26 Thread Andreas Preikschat
Review: Needs Fixing Hello Gerald, You need to provide error messages in lines 44 and 46 (if you do not provide one, the default one is taken, which says "Title and/or verses not found"). Why did you add RuntimeException to the sof but not to ooo? The ooo importer also has similar code in proce

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-25 Thread jerryb
I see one file made it. Let's try another one On Wed, May 25, 2011 at 4:58 PM, jerryb wrote: > I'm attaching two test files that fail silently in trunk but report > errors to the user with my patch.  Don't know if these attachments > will make it to launchpad or not > > On Wed, May 25, 2011 at

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-25 Thread jerryb
I'm attaching two test files that fail silently in trunk but report errors to the user with my patch. Don't know if these attachments will make it to launchpad or not On Wed, May 25, 2011 at 3:19 AM, Andreas Preikschat wrote: > Review: Needs Information > Do you have some files for us to test th

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-25 Thread jerryb
You can test it with anything you like: valid RTF (easy to make up), empty files (just touch empty.rtf and try it), completely wrong files (try to import the grep executable, if you like (may cause segfault in OpenOffice!)) On Wed, May 25, 2011 at 3:19 AM, Andreas Preikschat wrote: > Review: Need

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-25 Thread Andreas Preikschat
Review: Needs Information Do you have some files for us to test this? -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62211 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-24 Thread jerryb
@Jonathan -- done and done -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62211 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-24 Thread Jonathan Corwin
Review: Needs Fixing Line 24: This message needs to be in a translate() function. Also could it be changed to "Could not open OpenOffice.org or LibreOffice". Although connect may be correct from a technical point of view, the average end user might think it's trying to "connect" to the internet o

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-24 Thread jerryb
By the way I made the exception-catching explicit, following Python PEP-8 so as not to cover up other exceptions that might be undiscovered programming errors and SystemExit and KeyboardInterrupt exceptions among others. -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62211 Y

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-24 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: OpenLP Core (openlp-core) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/62211 This patch adds some robustness to the OpenOffice importer module and the Son

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-21 Thread noreply
The proposal to merge lp:~gerald-britton/openlp/newbugs into lp:openlp has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Your te

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-21 Thread Tim Bentley
The proposal to merge lp:~gerald-britton/openlp/newbugs into lp:openlp has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Y

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-21 Thread Tim Bentley
Review: Approve -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubs

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-21 Thread Raoul Snyman
Review: Approve -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubs

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-20 Thread jerryb
It's not calling log.error, but log_error. This causes a message to appear to the user. Currently, the import just fails silently. On Fri, May 20, 2011 at 12:59 PM, Tim Bentley wrote: > Review: Needs Information > would log.exception not give it to the user.  Log.error just writes it to the >

Re: [Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-20 Thread Tim Bentley
Review: Needs Information would log.exception not give it to the user. Log.error just writes it to the log file. Or display a popup? -- https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Your team OpenLP Core is subscribed to branch lp:openlp. ___

[Openlp-core] [Merge] lp:~gerald-britton/openlp/newbugs into lp:openlp

2011-05-20 Thread jerryb
jerryb has proposed merging lp:~gerald-britton/openlp/newbugs into lp:openlp. Requested reviews: OpenLP Core (openlp-core) For more details, see: https://code.launchpad.net/~gerald-britton/openlp/newbugs/+merge/61796 Fixed a bug in Words of Worship import where a file missing the WoW header i