Code review comment for lp:~trb143/openlp/ThemeManager

Revision history for this message
Michael Gorven (mgorven) wrote :

+ file=open(xmlfile)
+ xml =''.join(file.readlines()) # read the file and change list to a
string
+ file.close()
+ return xml

return open(xmlfile).read()

+ #print element.tag, element.text
(and others)

My preference is for debugging prints to either be removed completely, or
logged (possibly at a lower level than DEBUG).

+ words=words.replace(u'\r\n', u'\n')

I don't think this is the right way to deal with line endings. Did this
content come from a file?

+ verses_text=words.split(u'\n\n')

words.splitlines()

+ verses_text.append(u'\n'.join(v).lstrip()) # remove first \n

Why is the lstrip() needed? The join won't put a \n at the front.

+ #log.debug(u'_render_lines %s', lines)
(and others)

Again, either log or remove is my preference.

+ log.info(u'Items: %s' % self.items)
(and others)

log.info('Items: %s', self.items)

+ if file.endswith(u'.bmp'):

That will be case sensitive, which is probably not what we want.

 review needs_fixing

review: Needs Fixing

« Back to merge proposal