← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~trb143/openlp/next into lp:openlp

 

Review: Needs Fixing
154	+def checkDirectoryExists(dir):

This is a function, not a method attached to a Qt-derived object, it should be check_directory_exists.

161	+ log.debug(u'checkDirectoryExists')
162	+ if not os.path.exists(dir):
163	+ os.mkdir(dir)

If you're going to centralise this in a function, at least replicate the functionality properly. os.mkdir does not create non-existent parent directories, whereas os.makedirs does.

To be quite honest, your HtmlTags class is superfluous. You could merge all it's functionality into DisplayTags, and save some memory.

410	+ # cPickle only accepts str not unicode strings
411	+ user_tags = cPickle.loads(str(user_expands))

Be careful, if the user creates tags with non-ascii characters, this will break with an encoding issue. Rather use ``user_expands.encode(u'utf8')``


-- 
https://code.launchpad.net/~trb143/openlp/next/+merge/45607
Your team OpenLP Core is subscribed to branch lp:openlp.



References