← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~sfindlay/openlp/songs-import-powersong into lp:openlp

 

Nice work, I like byte puzzles ;-) . I cant test your code atm, but I have
a few comments below.

Line 231:
> +        The field label is separated from the field contents by one
random byte.

That byte isn't random! It actually specifies how long the string is in
characters (or bytes!)

Iirc, the SongShowPlus file format is similar.

Line 327:
Should the tab be invalid as well? I think we should keep the users
database clean from formatting. I think webkit will render this as a single
space.

Line 331:
Should this be moved to core/file/__unit__.py

I know this could be used else where. Iirc, some code I merged a couple
months back does.

Finally, I realise this might mean having to make extensive modifications
to the import wizard (so not suitable at this time), but could it be made
more user friendly by reading the xml file and giving the user a list of
song titles, rather than a list of unidentifiable file names. As I said,
maybe 2.1 or what ever is after the release?

Phill

-- 
https://code.launchpad.net/~sfindlay/openlp/songs-import-powersong/+merge/104102
Your team OpenLP Core is requested to review the proposed merge of lp:~sfindlay/openlp/songs-import-powersong into lp:openlp.


References