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

2012-03-23 Thread phill
Finally! I nearly cried last night lol On Mar 23, 2012 11:11 AM, wrote: > The proposal to merge lp:~phill-ridout/openlp/bug952533 into lp:openlp has > been updated. > >Status: Approved => Merged > > For more details, see: > https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/9897

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

2012-03-23 Thread noreply
The proposal to merge lp:~phill-ridout/openlp/bug952533 into lp:openlp has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 Your te

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

2012-03-23 Thread Jonathan Corwin
The proposal to merge lp:~phill-ridout/openlp/bug952533 into lp:openlp has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 Y

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

2012-03-23 Thread Jonathan Corwin
Review: Approve -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsub

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

2012-03-23 Thread Raoul Snyman
Review: Approve -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98975 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsub

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

2012-03-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug952533 into lp:openlp. Requested reviews: Jonathan Corwin (j-corwin) Raoul Snyman (raoul-snyman) Meinert Jordan (m2j) Related bugs: Bug #952533 in OpenLP: "Invalid file name characters in song name causes OpenLyrics to crash " https:

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

2012-03-22 Thread Jonathan Corwin
The r'' just prevents python string processing from handling \'s. It doesn't affect the regex. You still need to escape the special regex characters. However I think your original string might have worked, since python appears to leave in \'s that don't precede a special python shortcut, but's

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

2012-03-22 Thread phill
On 22 March 2012 21:22, Raoul Snyman wrote: > Review: Needs Fixing > > Uh, you just needed to change the u to an r. But if I'm using r'' i dont need to escape the other chars do I? http://docs.python.org/library/re.html#raw-string-notation Phill -- phill.rid...@googlemail.com - Google Talk ph

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

2012-03-22 Thread Raoul Snyman
Review: Needs Fixing Uh, you just needed to change the u to an r. -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98928 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Po

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

2012-03-22 Thread Raoul Snyman
In other words, it should look like this: INVALID_FILE_CHARS = re.compile(r'[\\\/:\*\?"<>\|\+\[\]%]', re.UNICODE) -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98928 Your team OpenLP Core is subscribed to branch lp:openlp. ___

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

2012-03-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug952533 into lp:openlp. Requested reviews: Meinert Jordan (m2j) Jonathan Corwin (j-corwin) Raoul Snyman (raoul-snyman) Related bugs: Bug #952533 in OpenLP: "Invalid file name characters in song name causes OpenLyrics to crash " https:

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

2012-03-22 Thread Raoul Snyman
Review: Needs Fixing 9 +INVALID_FILE_CHARS = re.compile(u'[\\\/:\*\?"<>\|\+\[\]%]', re.UNICODE) You need to use r'...' not u'...' if you want to actually use proper regular expressions. "r" means a raw string, so that backslashes are not interpreted as escape characters. -- https://code.

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

2012-03-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug952533 into lp:openlp. Requested reviews: Jonathan Corwin (j-corwin) Meinert Jordan (m2j) Related bugs: Bug #952533 in OpenLP: "Invalid file name characters in song name causes OpenLyrics to crash " https://bugs.launchpad.net/openlp/+b

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

2012-03-22 Thread Meinert Jordan
The plus is still missing. -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98885 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.

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

2012-03-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug952533 into lp:openlp. Requested reviews: Jonathan Corwin (j-corwin) Meinert Jordan (m2j) Related bugs: Bug #952533 in OpenLP: "Invalid file name characters in song name causes OpenLyrics to crash " https://bugs.launchpad.net/openlp/+b

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

2012-03-22 Thread Raoul Snyman
> Can I do something like from openlp.core.utils import CONTROL_CHARS? Yes. > I just thought the '| +$' was a bit more elegant than the other solution. > And anyways, I wanted to use my new found regular expression "skills" ;-) Either or, though I think using the Python method will be slightly f

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

2012-03-22 Thread phill
On Mar 22, 2012 8:50 AM, "Meinert Jordan" wrote: > Your list of reserved filename characters is incomplete. > For example the squared brackets are forbidden on FAT32 ( http://en.wikipedia.org/wiki/Filename). > The old list contains all reserved characters except of control characters. Thanks. I'v

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

2012-03-22 Thread Jonathan Corwin
You've created the constant CONTROL_CHARS but you've chosen not to use it on line 36. Perhaps you could use the constant and then just rstrip() to remove the rest instead of the '| +$'? If you don't want to use it on line 36, then why not merge the CONTROL_CHARS and INVALID_FILE_CHARS into one

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

2012-03-22 Thread Jonathan Corwin
Review: Needs Fixing Also there is an extra space before 'title' on line 36 -- https://code.launchpad.net/~phill-ridout/openlp/bug952533/+merge/98788 Your team OpenLP Core is subscribed to branch lp:openlp. ___ Mailing list: https://launchpad.net/~open

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

2012-03-22 Thread Meinert Jordan
Review: Needs Fixing Your list of reserved filename characters is incomplete. For example the squared brackets are forbidden on FAT32 (http://en.wikipedia.org/wiki/Filename). The old list contains all reserved characters except of control characters. -- https://code.launchpad.net/~phill-ridout/o

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

2012-03-22 Thread phill
phill has proposed merging lp:~phill-ridout/openlp/bug952533 into lp:openlp. Requested reviews: OpenLP Core (openlp-core) Related bugs: Bug #952533 in OpenLP: "Invalid file name characters in song name causes OpenLyrics to crash " https://bugs.launchpad.net/openlp/+bug/952533 For more deta