Merge lp:~tomasgroth/openlp/bugfixes16 into lp:openlp

[Tomas Groth]

Only update presentation thumbnails if needed. Fixes bug 1424330.
Make presentation controller save and read notes and titles in utf-8 encoding. Fixes bug 1424972.
Moved string format input outside calls to translate. Fixes bug 1425035.
Added option for check of format input strings in translation util script.
Removed tab from text since transifex cannot handle it.
Some PEP8 fixes (from pep8 1.6.1)
Use the language id when importing bibles.

[Tomas Groth (tomasgroth)]

lp:~tomasgroth/openlp/bugfixes16 (revision 2526)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/993/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/916/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/858/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/747/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/346/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/483/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/354/

[Raoul Snyman (raoul-snyman)]

This looks OK to me

review: Approve

[Tim Bentley (trb143)]

review: Approve

=== modified file 'openlp/core/ui/formattingtagcontroller.py'
--- openlp/core/ui/formattingtagcontroller.py	2015-01-18 13:39:21 +0000
+++ openlp/core/ui/formattingtagcontroller.py	2015-03-11 08:46:59 +0000
@@ -146,7 +146,7 @@
         end = self.start_html_to_end_html(start_html)
         if not end_html:
             if not end:
-                return translate('OpenLP.FormattingTagForm', 'Start tag %s is not valid HTML' % start_html), None
+                return translate('OpenLP.FormattingTagForm', 'Start tag %s is not valid HTML') % start_html, None
             return None, end
         return None, None
 
@@ -166,5 +166,5 @@
             return None, end
         if end and end != end_html:
             return translate('OpenLP.FormattingTagForm',
-                             'End tag %s does not match end tag for start tag %s' % (end, start_html)), None
+                             'End tag %s does not match end tag for start tag %s') % (end, start_html), None
         return None, None

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2015-02-10 22:25:30 +0000
+++ openlp/core/ui/servicemanager.py	2015-03-11 08:46:59 +0000
@@ -533,7 +533,7 @@
             self.application.set_normal_cursor()
             title = translate('OpenLP.ServiceManager', 'Service File(s) Missing')
             message = translate('OpenLP.ServiceManager',
-                                'The following file(s) in the service are missing:\n\t%s\n\n'
+                                'The following file(s) in the service are missing: %s\n\n'
                                 'These files will be removed if you continue to save.') % "\n\t".join(missing_list)
             answer = QtGui.QMessageBox.critical(self, title, message,
                                                 QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok |

=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2015-02-16 21:59:55 +0000
+++ openlp/core/ui/slidecontroller.py	2015-03-11 08:46:59 +0000
@@ -717,8 +717,8 @@
         self.play_slides_loop.setChecked(False)
         self.play_slides_loop.setIcon(build_icon(':/media/media_time.png'))
         if item.is_text():
-            if (Settings().value(self.main_window.songs_settings_section + '/display songbar')
-                    and not self.song_menu.menu().isEmpty()):
+            if (Settings().value(self.main_window.songs_settings_section + '/display songbar') and
+                    not self.song_menu.menu().isEmpty()):
                 self.toolbar.set_widget_visible(['song_menu'], True)
         if item.is_capable(ItemCapabilities.CanLoop) and len(item.get_frames()) > 1:
             self.toolbar.set_widget_visible(LOOP_LIST)

=== modified file 'openlp/plugins/alerts/lib/alertsmanager.py'
--- openlp/plugins/alerts/lib/alertsmanager.py	2015-02-27 23:02:19 +0000
+++ openlp/plugins/alerts/lib/alertsmanager.py	2015-03-11 08:46:59 +0000
@@ -70,8 +70,8 @@
         """
         Format and request the Alert and start the timer.
         """
-        if not self.alert_list or (self.live_controller.display.screens.display_count == 1
-                                   and not Settings().value('core/display on monitor')):
+        if not self.alert_list or (self.live_controller.display.screens.display_count == 1 and
+                                   not Settings().value('core/display on monitor')):
             return
         text = self.alert_list.pop(0)
         alert_tab = self.parent().settings_tab

=== modified file 'openlp/plugins/bibles/lib/opensong.py'
--- openlp/plugins/bibles/lib/opensong.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/bibles/lib/opensong.py	2015-03-11 08:46:59 +0000
@@ -123,8 +123,8 @@
                             verse_number += 1
                         self.create_verse(db_book.id, chapter_number, verse_number, self.get_text(verse))
                     self.wizard.increment_progress_bar(
-                        translate('BiblesPlugin.Opensong', 'Importing %(bookname)s %(chapter)s...' %
-                                  {'bookname': db_book.name, 'chapter': chapter_number}))
+                        translate('BiblesPlugin.Opensong', 'Importing %(bookname)s %(chapter)s...') %
+                        {'bookname': db_book.name, 'chapter': chapter_number})
                 self.session.commit()
             self.application.process_events()
         except etree.XMLSyntaxError as inst:

=== modified file 'openlp/plugins/bibles/lib/osis.py'
--- openlp/plugins/bibles/lib/osis.py	2015-02-02 20:40:31 +0000
+++ openlp/plugins/bibles/lib/osis.py	2015-03-11 08:46:59 +0000
@@ -71,6 +71,7 @@
             if not language_id:
                 log.error('Importing books from "%s" failed' % self.filename)
                 return False
+            self.save_meta('language_id', language_id)
             num_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=namespace))
             self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport',
                                                          'Removing unused tags (this may take a few minutes)...'))
@@ -124,7 +125,7 @@
                     break
                 # Remove div-tags in the book
                 etree.strip_tags(book, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}div'))
-                book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books)
+                book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books, language_id)
                 if not book_ref_id:
                     book_ref_id = self.get_book_ref_id_by_localised_name(book.get('osisID'))
                 if not book_ref_id:
@@ -154,8 +155,8 @@
                                 verse_number = verse.get("osisID").split('.')[2]
                                 self.create_verse(db_book.id, chapter_number, verse_number, verse.text.strip())
                         self.wizard.increment_progress_bar(
-                            translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...' %
-                                      {'bookname': db_book.name, 'chapter': chapter_number}))
+                            translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') %
+                            {'bookname': db_book.name, 'chapter': chapter_number})
                 else:
                     # The chapter tags is used as milestones. For now we assume verses is also milestones
                     chapter_number = 0
@@ -164,8 +165,8 @@
                                 and element.get('sID'):
                             chapter_number = element.get("osisID").split('.')[1]
                             self.wizard.increment_progress_bar(
-                                translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...' %
-                                          {'bookname': db_book.name, 'chapter': chapter_number}))
+                                translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') %
+                                {'bookname': db_book.name, 'chapter': chapter_number})
                         elif element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse' \
                                 and element.get('sID'):
                             # If this tag marks the start of a verse, the verse text is between this tag and

=== modified file 'openlp/plugins/bibles/lib/zefania.py'
--- openlp/plugins/bibles/lib/zefania.py	2015-02-02 20:40:31 +0000
+++ openlp/plugins/bibles/lib/zefania.py	2015-03-11 08:46:59 +0000
@@ -69,6 +69,7 @@
             if not language_id:
                 log.error('Importing books from "%s" failed' % self.filename)
                 return False
+            self.save_meta('language_id', language_id)
             num_books = int(zefania_bible_tree.xpath("count(//BIBLEBOOK)"))
             # Strip tags we don't use - keep content
             etree.strip_tags(zefania_bible_tree, ('STYLE', 'GRAM', 'NOTE', 'SUP', 'XREF'))
@@ -83,7 +84,7 @@
                 if not bname and not bnumber:
                     continue
                 if bname:
-                    book_ref_id = self.get_book_ref_id_by_name(bname, num_books)
+                    book_ref_id = self.get_book_ref_id_by_name(bname, num_books, language_id)
                     if not book_ref_id:
                         book_ref_id = self.get_book_ref_id_by_localised_name(bname)
                 else:
@@ -102,8 +103,8 @@
                         verse_number = VERS.get("vnumber")
                         self.create_verse(db_book.id, chapter_number, verse_number, VERS.text.replace('<BR/>', '\n'))
                     self.wizard.increment_progress_bar(
-                        translate('BiblesPlugin.Zefnia', 'Importing %(bookname)s %(chapter)s...' %
-                                  {'bookname': db_book.name, 'chapter': chapter_number}))
+                        translate('BiblesPlugin.Zefnia', 'Importing %(bookname)s %(chapter)s...') %
+                        {'bookname': db_book.name, 'chapter': chapter_number})
             self.session.commit()
             self.application.process_events()
         except Exception as e:

=== modified file 'openlp/plugins/presentations/lib/mediaitem.py'
--- openlp/plugins/presentations/lib/mediaitem.py	2015-01-27 14:59:06 +0000
+++ openlp/plugins/presentations/lib/mediaitem.py	2015-03-11 08:46:59 +0000
@@ -230,17 +230,26 @@
             Settings().setValue(self.settings_section + '/presentations files', self.get_file_list())
             self.application.set_normal_cursor()
 
-    def clean_up_thumbnails(self, filepath):
+    def clean_up_thumbnails(self, filepath, clean_for_update=False):
         """
         Clean up the files created such as thumbnails
 
         :param filepath: File path of the presention to clean up after
+        :param clean_for_update: Only clean thumbnails if update is needed
         :return: None
         """
         for cidx in self.controllers:
-            doc = self.controllers[cidx].add_document(filepath)
-            doc.presentation_deleted()
-            doc.close_presentation()
+            root, file_ext = os.path.splitext(filepath)
+            file_ext = file_ext[1:]
+            if file_ext in self.controllers[cidx].supports or file_ext in self.controllers[cidx].also_supports:
+                doc = self.controllers[cidx].add_document(filepath)
+                if clean_for_update:
+                    thumb_path = doc.get_thumbnail_path(1, True)
+                    if not thumb_path or os.path.getmtime(thumb_path) < os.path.getmtime(filepath):
+                        doc.presentation_deleted()
+                else:
+                    doc.presentation_deleted()
+                doc.close_presentation()
 
     def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
                             context=ServiceItemContext.Service, presentation_file=None):

=== modified file 'openlp/plugins/presentations/lib/presentationcontroller.py'
--- openlp/plugins/presentations/lib/presentationcontroller.py	2015-02-10 21:01:22 +0000
+++ openlp/plugins/presentations/lib/presentationcontroller.py	2015-03-11 08:46:59 +0000
@@ -132,7 +132,7 @@
         """
         The location where thumbnail images will be stored
         """
-        # TODO: If statment can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(self.file_path.encode('utf-8'))
         else:
@@ -143,7 +143,7 @@
         """
         The location where thumbnail images will be stored
         """
-        # TODO: If statment can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(self.file_path.encode('utf-8'))
         else:
@@ -306,7 +306,7 @@
         titles_file = os.path.join(self.get_thumbnail_folder(), 'titles.txt')
         if os.path.exists(titles_file):
             try:
-                with open(titles_file) as fi:
+                with open(titles_file, encoding='utf-8') as fi:
                     titles = fi.read().splitlines()
             except:
                 log.exception('Failed to open/read existing titles file')
@@ -316,7 +316,7 @@
             note = ''
             if os.path.exists(notes_file):
                 try:
-                    with open(notes_file) as fn:
+                    with open(notes_file, encoding='utf-8') as fn:
                         note = fn.read()
                 except:
                     log.exception('Failed to open/read notes file')
@@ -331,12 +331,12 @@
         """
         if titles:
             titles_file = os.path.join(self.get_thumbnail_folder(), 'titles.txt')
-            with open(titles_file, mode='w') as fo:
+            with open(titles_file, mode='wt', encoding='utf-8') as fo:
                 fo.writelines(titles)
         if notes:
             for slide_no, note in enumerate(notes, 1):
                 notes_file = os.path.join(self.get_thumbnail_folder(), 'slideNotes%d.txt' % slide_no)
-                with open(notes_file, mode='w') as fn:
+                with open(notes_file, mode='wt', encoding='utf-8') as fn:
                     fn.write(note)
 
 

=== modified file 'openlp/plugins/presentations/presentationplugin.py'
--- openlp/plugins/presentations/presentationplugin.py	2015-01-27 19:57:09 +0000
+++ openlp/plugins/presentations/presentationplugin.py	2015-03-11 08:46:59 +0000
@@ -144,7 +144,7 @@
         files_from_config = Settings().value('presentations/presentations files')
         for file in files_from_config:
             try:
-                self.media_item.clean_up_thumbnails(file)
+                self.media_item.clean_up_thumbnails(file, True)
             except AttributeError:
                 pass
         self.media_item.list_view.clear()

=== modified file 'openlp/plugins/songs/lib/importers/worshipassistant.py'
--- openlp/plugins/songs/lib/importers/worshipassistant.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/songs/lib/importers/worshipassistant.py	2015-03-11 08:46:59 +0000
@@ -179,6 +179,6 @@
                         cleaned_verse_order_list.append(verse)
                 self.verse_order_list = cleaned_verse_order_list
             if not self.finish():
-                self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d') % index
-                               + (': "' + self.title + '"' if self.title else ''))
+                self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d') % index +
+                               (': "' + self.title + '"' if self.title else ''))
             songs_file.close()

=== modified file 'openlp/plugins/songs/lib/importers/zionworx.py'
--- openlp/plugins/songs/lib/importers/zionworx.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/songs/lib/importers/zionworx.py	2015-03-11 08:46:59 +0000
@@ -118,8 +118,8 @@
                     self.add_verse(verse)
                 title = self.title
                 if not self.finish():
-                    self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index
-                                   + (': "' + title + '"' if title else ''))
+                    self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index +
+                                   (': "' + title + '"' if title else ''))
 
     def _decode(self, str):
         """

=== modified file 'scripts/translation_utils.py'
--- scripts/translation_utils.py	2015-01-22 18:40:12 +0000
+++ scripts/translation_utils.py	2015-03-11 08:46:59 +0000
@@ -53,7 +53,9 @@
 import base64
 import json
 import webbrowser
+import glob
 
+from lxml import etree, objectify
 from optparse import OptionParser
 from PyQt4 import QtCore
 
@@ -76,6 +78,7 @@
     Prepare = 3
     Update = 4
     Generate = 5
+    Check = 6
 
 
 class CommandStack(object):
@@ -292,6 +295,38 @@
     print_quiet('Opening browser to OpenLP project...')
 
 
+def check_format_strings():
+    """
+    This method runs through the ts-files and looks for mismatches between format strings in the original text
+    and in the translations.
+    """
+    path = os.path.join(os.path.abspath('..'), 'resources', 'i18n', '*.ts')
+    file_list = glob.glob(path)
+    for filename in file_list:
+        print_quiet('Checking %s' % filename)
+        file = open(filename, 'rb')
+        tree = objectify.parse(file)
+        root = tree.getroot()
+        for tag in root.iter('message'):
+            location = tag.location.get('filename')
+            line = tag.location.get('line')
+            org_text = tag.source.text
+            translation = tag.translation.text
+            if not translation:
+                for num in tag.iter('numerusform'):
+                    print_verbose('parsed numerusform: location: %s, source: %s, translation: %s' % (
+                                  location, org_text, num.text))
+                    if num and org_text.count('%') != num.text.count('%'):
+                        print_quiet(
+                            'ERROR: Translation from %s at line %s has a mismatch of format input:\n%s\n%s\n' % (
+                                location, line, org_text, num.text))
+            else:
+                print_verbose('parsed: location: %s, source: %s, translation: %s' % (location, org_text, translation))
+                if org_text.count('%') != translation.count('%'):
+                    print_quiet('ERROR: Translation from %s at line %s has a mismatch of format input:\n%s\n%s\n' % (
+                                location, line, org_text, translation))
+
+
 def process_stack(command_stack):
     """
     This method looks at the commands in the command stack, and processes them
@@ -315,6 +350,8 @@
                 generate_binaries()
             elif command == Command.Create:
                 create_translation()
+            elif command == Command.Check:
+                check_format_strings()
         print_quiet('Finished processing commands.')
     else:
         print_quiet('No commands to process.')
@@ -345,6 +382,8 @@
                       help='show extra information while processing translations')
     parser.add_option('-q', '--quiet', dest='quiet', action='store_true',
                       help='suppress all output other than errors')
+    parser.add_option('-f', '--check-format-strings', dest='check', action='store_true',
+                      help='check that format strings are matching in translations')
     (options, args) = parser.parse_args()
     # Create and populate the command stack
     command_stack = CommandStack()
@@ -358,6 +397,8 @@
         command_stack.append(Command.Update)
     if options.generate:
         command_stack.append(Command.Generate)
+    if options.check:
+        command_stack.append(Command.Check)
     verbose_mode = options.verbose
     quiet_mode = options.quiet
     if options.username:

=== modified file 'tests/functional/openlp_plugins/presentations/test_mediaitem.py'
--- tests/functional/openlp_plugins/presentations/test_mediaitem.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/presentations/test_mediaitem.py	2015-03-11 08:46:59 +0000
@@ -26,7 +26,7 @@
 
 from openlp.core.common import Registry
 from openlp.plugins.presentations.lib.mediaitem import PresentationMediaItem
-from tests.functional import patch, MagicMock
+from tests.functional import patch, MagicMock, call
 from tests.helpers.testmixin import TestMixin
 
 
@@ -84,3 +84,26 @@
         self.assertIn('*.pdf', self.media_item.on_new_file_masks, 'The file mask should contain the pdf extension')
         self.assertIn('*.xps', self.media_item.on_new_file_masks, 'The file mask should contain the xps extension')
         self.assertIn('*.oxps', self.media_item.on_new_file_masks, 'The file mask should contain the oxps extension')
+
+    def clean_up_thumbnails_test(self):
+        """
+        Test that the clean_up_thumbnails method works as expected.
+        """
+        # GIVEN: A mocked controller, and mocked os.path.getmtime
+        mocked_controller = MagicMock()
+        mocked_doc = MagicMock()
+        mocked_controller.add_document.return_value = mocked_doc
+        mocked_controller.supports = ['tmp']
+        self.media_item.controllers = {
+            'Mocked': mocked_controller
+        }
+        presentation_file = 'file.tmp'
+        with patch('openlp.plugins.presentations.lib.mediaitem.os.path.getmtime') as mocked_getmtime:
+            mocked_getmtime.side_effect = [100, 200]
+
+            # WHEN: calling clean_up_thumbnails
+            self.media_item.clean_up_thumbnails(presentation_file, True)
+
+        # THEN: doc.presentation_deleted should have been called since the thumbnails mtime will be greater than
+        #       the presentation_file's mtime.
+        mocked_doc.assert_has_calls([call.get_thumbnail_path(1, True), call.presentation_deleted()], True)

=== modified file 'tests/functional/openlp_plugins/presentations/test_presentationcontroller.py'
--- tests/functional/openlp_plugins/presentations/test_presentationcontroller.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/presentations/test_presentationcontroller.py	2015-03-11 08:46:59 +0000
@@ -81,9 +81,9 @@
             self.document.save_titles_and_notes(titles, notes)
 
             # THEN: the last call to open should have been for slideNotes2.txt
-            mocked_open.assert_any_call(os.path.join('test', 'titles.txt'), mode='w')
-            mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt'), mode='w')
-            mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt'), mode='w')
+            mocked_open.assert_any_call(os.path.join('test', 'titles.txt'), mode='wt', encoding='utf-8')
+            mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt'), mode='wt', encoding='utf-8')
+            mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt'), mode='wt', encoding='utf-8')
             self.assertEqual(mocked_open.call_count, 3, 'There should be exactly three files opened')
             mocked_open().writelines.assert_called_once_with(['uno', 'dos'])
             mocked_open().write.assert_called_any('one')
@@ -126,9 +126,9 @@
             self.assertIs(type(result_notes), list, 'result_notes should be of type list')
             self.assertEqual(len(result_notes), 2, 'There should be two items in the notes')
             self.assertEqual(mocked_open.call_count, 3, 'Three files should be opened')
-            mocked_open.assert_any_call(os.path.join('test', 'titles.txt'))
-            mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt'))
-            mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt'))
+            mocked_open.assert_any_call(os.path.join('test', 'titles.txt'), encoding='utf-8')
+            mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt'), encoding='utf-8')
+            mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt'), encoding='utf-8')
             self.assertEqual(mocked_exists.call_count, 3, 'Three files should have been checked')
 
     def get_titles_and_notes_with_file_not_found_test(self):