← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~alisonken1/openlp/strings-templates into lp:openlp

 

Ken Roberts has proposed merging lp:~alisonken1/openlp/strings-templates into lp:openlp.

Commit message:
String conversions with templates part 1

Requested reviews:
  Tim Bentley (trb143)

For more details, see:
https://code.launchpad.net/~alisonken1/openlp/strings-templates/+merge/296486

String conversions with templates part 1

- Fix string format key error in first time wizard from previous string conversion
- Fix projector pjlink1 test to use MagicMock
- Part 1 string conversions where format template is a string variable that's filled later
- Update projectordb test
- Fix a test on htmlbuilder string formatting
- Revert openlp/core/lib/htmlbuilder.py

--------------------------------
lp:~alisonken1/openlp/strings-templates (revision 2676)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1589/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1500/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1438/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1217/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/807/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/875/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/743/

-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/common/uistrings.py'
--- openlp/core/common/uistrings.py	2016-05-14 04:24:46 +0000
+++ openlp/core/common/uistrings.py	2016-06-04 08:15:27 +0000
@@ -80,8 +80,9 @@
         self.Export = translate('OpenLP.Ui', 'Export')
         self.File = translate('OpenLP.Ui', 'File')
         self.FileNotFound = translate('OpenLP.Ui', 'File Not Found')
-        # TODO: Check before converting to python3 string
-        self.FileNotFoundMessage = translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')
+        # TODO: Passed nose - verify in real life example
+        self.FileNotFoundMessage = translate('OpenLP.Ui',
+                                             'File {name} not found.\nPlease try selecting it individually.')
         self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit')
         self.Help = translate('OpenLP.Ui', 'Help')
         self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours')
@@ -140,8 +141,8 @@
         self.Split = translate('OpenLP.Ui', 'Optional &Split')
         self.SplitToolTip = translate('OpenLP.Ui',
                                       'Split a slide into two only if it does not fit on the screen as one slide.')
-        # TODO: Check before converting to python3 string
-        self.StartTimeCode = translate('OpenLP.Ui', 'Start %s')
+        # TODO: WHERE is this used at? cannot find where it's used at in code.
+        self.StartTimeCode = translate('OpenLP.Ui', 'Start {code}')
         self.StopPlaySlidesInLoop = translate('OpenLP.Ui', 'Stop Play Slides in Loop')
         self.StopPlaySlidesToEnd = translate('OpenLP.Ui', 'Stop Play Slides to End')
         self.Theme = translate('OpenLP.Ui', 'Theme', 'Singular')

=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py	2016-05-21 05:02:31 +0000
+++ openlp/core/lib/__init__.py	2016-06-04 08:15:27 +0000
@@ -323,8 +323,7 @@
         return ''
     elif len(string_list) == 1:
         return string_list[0]
-    # TODO:
-    #   Cannot convert these strings to python3 yet until I can figure out how to mock translate() with the new format
+    # TODO: Verify mocking of translate() test before conversion
     elif len(string_list) == 2:
         return translate('OpenLP.core.lib', '%s and %s',
                          'Locale list separator: 2 items') % (string_list[0], string_list[1])

=== modified file 'openlp/core/lib/filedialog.py'
--- openlp/core/lib/filedialog.py	2016-05-15 17:33:42 +0000
+++ openlp/core/lib/filedialog.py	2016-06-04 08:15:27 +0000
@@ -51,9 +51,9 @@
                 file = parse.unquote(file)
                 if not os.path.exists(file):
                     log.error('File {text} not found.'.format(text=file))
-                    # TODO: Test with UiStrings() before converting to python3 strings
+                    # TODO: Should work - need to verify
                     QtWidgets.QMessageBox.information(parent, UiStrings().FileNotFound,
-                                                      UiStrings().FileNotFoundMessage % file)
+                                                      UiStrings().FileNotFoundMessage.format(name=file))
                     continue
             file_list.append(file)
         return file_list

=== modified file 'openlp/core/lib/renderer.py'
--- openlp/core/lib/renderer.py	2016-05-15 17:33:42 +0000
+++ openlp/core/lib/renderer.py	2016-06-04 08:15:27 +0000
@@ -370,21 +370,22 @@
         self.web.resize(self.page_width, self.page_height)
         self.web_frame = self.web.page().mainFrame()
         # Adjust width and height to account for shadow. outline done in css.
-        # TODO: Verify before converting to python3 strings
+        # TODO: Tested at home
         html = """<!DOCTYPE html><html><head><script>
-            function show_text(newtext) {
+            function show_text(newtext) {{
                 var main = document.getElementById('main');
                 main.innerHTML = newtext;
                 // We need to be sure that the page is loaded, that is why we
                 // return the element's height (even though we do not use the
                 // returned value).
                 return main.offsetHeight;
-            }
-            </script><style>*{margin: 0; padding: 0; border: 0;}
-            #main {position: absolute; top: 0px; %s %s}</style></head><body>
-            <div id="main"></div></body></html>""" % \
-            (build_lyrics_format_css(theme_data, self.page_width, self.page_height),
-             build_lyrics_outline_css(theme_data))
+            }}
+            </script><style>*{{margin: 0; padding: 0; border: 0;}}
+            #main {{position: absolute; top: 0px; {format_css} {outline_css}}}</style></head><body>
+            <div id="main"></div></body></html>""".format(format_css=build_lyrics_format_css(theme_data,
+                                                                                             self.page_width,
+                                                                                             self.page_height),
+                                                          outline_css=build_lyrics_outline_css(theme_data))
         self.web.setHtml(html)
         self.empty_height = self.web_frame.contentsSize().height()
 

=== modified file 'openlp/core/lib/theme.py'
--- openlp/core/lib/theme.py	2016-05-15 17:33:42 +0000
+++ openlp/core/lib/theme.py	2016-06-04 08:15:27 +0000
@@ -513,8 +513,8 @@
         theme_strings = []
         for key in dir(self):
             if key[0:1] != '_':
-                # TODO: Verify spacing format before converting to python3 string
-                theme_strings.append('%30s: %s' % (key, getattr(self, key)))
+                # TODO: Tested at home
+                theme_strings.append('{key:>30}: {value}'.format(key=key, value=getattr(self, key)))
         return '\n'.join(theme_strings)
 
     def _build_xml_from_attrs(self):

=== modified file 'openlp/core/ui/exceptionform.py'
--- openlp/core/ui/exceptionform.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/exceptionform.py	2016-06-04 08:15:27 +0000
@@ -91,13 +91,13 @@
         super(ExceptionForm, self).__init__(None, QtCore.Qt.WindowSystemMenuHint | QtCore.Qt.WindowTitleHint)
         self.setupUi(self)
         self.settings_section = 'crashreport'
-        # TODO: Need to see how to format strings when string with tags is actually a variable
+        # TODO: Should work - need to test
         self.report_text = '**OpenLP Bug Report**\n' \
-            'Version: %s\n\n' \
-            '--- Details of the Exception. ---\n\n%s\n\n ' \
-            '--- Exception Traceback ---\n%s\n' \
-            '--- System information ---\n%s\n' \
-            '--- Library Versions ---\n%s\n'
+            'Version: {version}\n\n' \
+            '--- Details of the Exception. ---\n\n{description}\n\n ' \
+            '--- Exception Traceback ---\n{traceback}\n' \
+            '--- System information ---\n{system}\n' \
+            '--- Library Versions ---\n{libs}\n'
 
     def exec(self):
         """
@@ -133,7 +133,15 @@
                 system += 'Desktop: GNOME\n'
             elif os.environ.get('DESKTOP_SESSION') == 'xfce':
                 system += 'Desktop: Xfce\n'
-        return openlp_version, description, traceback, system, libraries
+        # NOTE: This needs to return a string that format() will use. See __init__.self.report_text for names.
+        return ("version='{version}', "
+                "description='{description}', "
+                "traceback='{traceback}', "
+                "libs='{libs}'").format(version=openlp_version,
+                                        description=description,
+                                        traceback=traceback,
+                                        system=system,
+                                        libs=libraries)
 
     def on_save_report_button_clicked(self):
         """
@@ -147,7 +155,8 @@
         if filename:
             filename = str(filename).replace('/', os.path.sep)
             Settings().setValue(self.settings_section + '/last directory', os.path.dirname(filename))
-            report_text = self.report_text % self._create_report()
+            # NOTE: self._create_report() should return a string with the key names for format()
+            report_text = self.report_text.format(self._create_report())
             try:
                 report_file = open(filename, 'w')
                 try:
@@ -167,6 +176,7 @@
         """
         Opening systems default email client and inserting exception log and system information.
         """
+        # NOTE: self._create_report() should return a string with keys for format()
         content = self._create_report()
         source = ''
         exception = ''
@@ -178,8 +188,8 @@
         subject = 'Bug report: {error} in {source}'.format(error=exception, source=source)
         mail_urlquery = QtCore.QUrlQuery()
         mail_urlquery.addQueryItem('subject', subject)
-        # TODO: Find out how to format() text that is in a variable
-        mail_urlquery.addQueryItem('body', self.report_text % content)
+        # TODO: Should be good - need to test
+        mail_urlquery.addQueryItem('body', self.report_text.format(content))
         if self.file_attachment:
             mail_urlquery.addQueryItem('attach', self.file_attachment)
         mail_to_url = QtCore.QUrl('mailto:bugs@xxxxxxxxxx')

=== modified file 'openlp/core/ui/firsttimeform.py'
--- openlp/core/ui/firsttimeform.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/firsttimeform.py	2016-06-04 08:15:27 +0000
@@ -207,8 +207,8 @@
                 trace_error_handler(log)
         self.update_screen_list_combo()
         self.application.process_events()
-        # TODO: Figure out how to use a variable with format()
-        self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading %s...')
+        # TODO: Tested at home
+        self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading {name}...')
         if self.has_run_wizard:
             self.songs_check_box.setChecked(self.plugin_manager.get_plugin_by_name('songs').is_active())
             self.bible_check_box.setChecked(self.plugin_manager.get_plugin_by_name('bibles').is_active())
@@ -564,7 +564,7 @@
             self.progress_bar.setValue(self.progress_bar.maximum())
             if self.has_run_wizard:
                 text = translate('OpenLP.FirstTimeWizard',
-                                 'Download complete. Click the {button} button to return to OpenLP.'
+                                 'Download complete. Click the {text} button to return to OpenLP.'
                                  ).format(text=clean_button_text(self.buttonText(QtWidgets.QWizard.FinishButton)))
                 self.progress_label.setText(text)
             else:
@@ -632,7 +632,8 @@
             item = self.songs_list_widget.item(i)
             if item.checkState() == QtCore.Qt.Checked:
                 filename, sha256 = item.data(QtCore.Qt.UserRole)
-                self._increment_progress_bar(self.downloading % filename, 0)
+                # TODO: Tested at home
+                self._increment_progress_bar(self.downloading.format(name=filename), 0)
                 self.previous_size = 0
                 destination = os.path.join(songs_destination, str(filename))
                 if not self.url_get_file('{path}{name}'.format(path=self.songs_url, name=filename),
@@ -644,7 +645,8 @@
             item = bibles_iterator.value()
             if item.parent() and item.checkState(0) == QtCore.Qt.Checked:
                 bible, sha256 = item.data(0, QtCore.Qt.UserRole)
-                self._increment_progress_bar(self.downloading % bible, 0)
+                # TODO: Tested at home
+                self._increment_progress_bar(self.downloading.format(name=bible), 0)
                 self.previous_size = 0
                 if not self.url_get_file('{path}{name}'.format(path=self.bibles_url, name=bible),
                                          os.path.join(bibles_destination, bible),
@@ -656,8 +658,8 @@
             item = self.themes_list_widget.item(i)
             if item.checkState() == QtCore.Qt.Checked:
                 theme, sha256 = item.data(QtCore.Qt.UserRole)
-                # TODO: Verify how to use format() with strings in a variable
-                self._increment_progress_bar(self.downloading % theme, 0)
+                # TODO: Tested at home
+                self._increment_progress_bar(self.downloading.format(name=theme), 0)
                 self.previous_size = 0
                 if not self.url_get_file('{path}{name}'.format(path=self.themes_url, name=theme),
                                          os.path.join(themes_destination, theme),

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/mainwindow.py	2016-06-04 08:15:27 +0000
@@ -1334,7 +1334,7 @@
         self.recent_files_menu.clear()
         for file_id, filename in enumerate(recent_files_to_display):
             log.debug('Recent file name: {name}'.format(name=filename))
-            # TODO: Verify ''.format() before committing
+            # TODO: Should be good
             action = create_action(self, '',
                                    text='&{n} {name}'.format(n=file_id + 1,
                                                              name=os.path.splitext(os.path.basename(str(filename)))[0]),

=== modified file 'openlp/core/ui/pluginform.py'
--- openlp/core/ui/pluginform.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/pluginform.py	2016-06-04 08:15:27 +0000
@@ -60,7 +60,7 @@
         self._clear_details()
         self.programatic_change = True
         plugin_list_width = 0
-        # TODO: See how to use format() with variables
+        # TODO: Tested at home
         for plugin in self.plugin_manager.plugins:
             item = QtWidgets.QListWidgetItem(self.plugin_list_widget)
             # We do this just to make 100% sure the status is an integer as
@@ -68,19 +68,19 @@
             plugin.status = int(plugin.status)
             # Set the little status text in brackets next to the plugin name.
             if plugin.status == PluginStatus.Disabled:
-                status_text = translate('OpenLP.PluginForm', '%s (Disabled)')
+                status_text = translate('OpenLP.PluginForm', '{name} (Disabled)')
             elif plugin.status == PluginStatus.Active:
-                status_text = translate('OpenLP.PluginForm', '%s (Active)')
+                status_text = translate('OpenLP.PluginForm', '{name} (Active)')
             else:
                 # PluginStatus.Inactive
-                status_text = translate('OpenLP.PluginForm', '%s (Inactive)')
-            item.setText(status_text % plugin.name_strings['singular'])
+                status_text = translate('OpenLP.PluginForm', '{name} (Inactive)')
+            item.setText(status_text.format(name=plugin.name_strings['singular']))
             # If the plugin has an icon, set it!
             if plugin.icon:
                 item.setIcon(plugin.icon)
             self.plugin_list_widget.addItem(item)
             plugin_list_width = max(plugin_list_width, self.fontMetrics().width(
-                translate('OpenLP.PluginForm', '%s (Inactive)') % plugin.name_strings['singular']))
+                translate('OpenLP.PluginForm', '{name} (Inactive)').format(name=plugin.name_strings['singular'])))
         self.plugin_list_widget.setFixedWidth(plugin_list_width + self.plugin_list_widget.iconSize().width() + 48)
 
     def _clear_details(self):
@@ -137,13 +137,13 @@
             self.active_plugin.app_startup()
         else:
             self.active_plugin.toggle_status(PluginStatus.Inactive)
-        # TODO: Verify using format() with a variable
-        status_text = translate('OpenLP.PluginForm', '%s (Inactive)')
+        # TODO: Tested at home
+        status_text = translate('OpenLP.PluginForm', '{name} (Inactive)')
         if self.active_plugin.status == PluginStatus.Active:
-            status_text = translate('OpenLP.PluginForm', '%s (Active)')
+            status_text = translate('OpenLP.PluginForm', '{name} (Active)')
         elif self.active_plugin.status == PluginStatus.Inactive:
-            status_text = translate('OpenLP.PluginForm', '%s (Inactive)')
+            status_text = translate('OpenLP.PluginForm', '{name} (Inactive)')
         elif self.active_plugin.status == PluginStatus.Disabled:
-            status_text = translate('OpenLP.PluginForm', '%s (Disabled)')
+            status_text = translate('OpenLP.PluginForm', '{name} (Disabled)')
         self.plugin_list_widget.currentItem().setText(
-            status_text % self.active_plugin.name_strings['singular'])
+            status_text.format(name=self.active_plugin.name_strings['singular']))

=== modified file 'openlp/core/ui/themeform.py'
--- openlp/core/ui/themeform.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/themeform.py	2016-06-04 08:15:27 +0000
@@ -464,9 +464,9 @@
         """
         Background video button pushed.
         """
-        # TODO: Check this before converting
-        visible_formats = '(%s)' % '; '.join(VIDEO_EXT)
-        actual_formats = '(%s)' % ' '.join(VIDEO_EXT)
+        # TODO: Should work
+        visible_formats = '({name})'.format(name='; '.join(VIDEO_EXT))
+        actual_formats = '({name})'.format(name=' '.join(VIDEO_EXT))
         video_filter = '{trans} {visible} {actual}'.format(trans=translate('OpenLP', 'Video Files'),
                                                            visible=visible_formats, actual=actual_formats)
         video_filter = '{video};;{ui} (*.*)'.format(video=video_filter, ui=UiStrings().AllFiles)

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2016-05-20 16:22:06 +0000
+++ openlp/core/ui/thememanager.py	2016-06-04 08:15:27 +0000
@@ -769,7 +769,7 @@
                                                                              '{count} time(s) by {plugin}'
                                                                              ).format(name=used_count,
                                                                                       plugin=plugin.name)))
-                        plugin_usage = "%s\n" % plugin_usage
+                        plugin_usage = "{text}\n".format(text=plugin_usage)
                 if plugin_usage:
                     critical_error_message_box(translate('OpenLP.ThemeManager', 'Unable to delete theme'),
                                                translate('OpenLP.ThemeManager',

=== modified file 'tests/functional/openlp_core_common/test_registryproperties.py'
--- tests/functional/openlp_core_common/test_registryproperties.py	2016-05-31 21:40:13 +0000
+++ tests/functional/openlp_core_common/test_registryproperties.py	2016-06-04 08:15:27 +0000
@@ -75,4 +75,3 @@
 
         # THEN the application should be none
         self.assertEqual(self.application, application, 'The application value should match')
-

=== modified file 'tests/functional/openlp_core_lib/test_file_dialog.py'
--- tests/functional/openlp_core_lib/test_file_dialog.py	2016-05-31 21:40:13 +0000
+++ tests/functional/openlp_core_lib/test_file_dialog.py	2016-06-04 08:15:27 +0000
@@ -60,7 +60,7 @@
         self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [
             '/Valid File', '/url encoded file #1']
         self.mocked_ui_strings().FileNotFound = 'File Not Found'
-        self.mocked_ui_strings().FileNotFoundMessage = 'File %s not found.\nPlease try selecting it individually.'
+        self.mocked_ui_strings().FileNotFoundMessage = 'File {name} not found.\nPlease try selecting it individually.'
 
         # WHEN: FileDialog.getOpenFileNames is called
         result = FileDialog.getOpenFileNames(self.mocked_parent)

=== modified file 'tests/functional/openlp_core_lib/test_projector_pjlink1.py'
--- tests/functional/openlp_core_lib/test_projector_pjlink1.py	2016-05-31 21:40:13 +0000
+++ tests/functional/openlp_core_lib/test_projector_pjlink1.py	2016-06-04 08:15:27 +0000
@@ -29,26 +29,12 @@
 from openlp.core.lib.projector.constants import E_PARAMETER, ERROR_STRING, S_OFF, S_STANDBY, S_WARMUP, S_ON, \
     S_COOLDOWN, PJLINK_POWR_STATUS
 
-from tests.functional import patch
+from tests.functional import MagicMock, patch
 from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE
 
 pjlink_test = PJLink1(name='test', ip='127.0.0.1', pin=TEST_PIN, no_poll=True)
 
 
-class DummyTimer(object):
-    '''
-    Dummy class to fake timers
-    '''
-    def __init__(self, *args, **kwargs):
-        pass
-
-    def start(self, *args, **kwargs):
-        pass
-
-    def stop(self, *args, **kwargs):
-        pass
-
-
 class TestPJLink(TestCase):
     """
     Tests for the PJLink module
@@ -308,8 +294,8 @@
         pjlink.other_info = 'ANOTHER TEST'
         pjlink.send_queue = True
         pjlink.send_busy = True
-        pjlink.timer = DummyTimer()
-        pjlink.socket_timer = DummyTimer()
+        pjlink.timer = MagicMock()
+        pjlink.socket_timer = MagicMock()
 
         # WHEN: reset_information() is called
         with patch.object(pjlink.timer, 'stop') as mock_timer:

=== modified file 'tests/functional/openlp_core_lib/test_projectordb.py'
--- tests/functional/openlp_core_lib/test_projectordb.py	2016-05-31 21:40:13 +0000
+++ tests/functional/openlp_core_lib/test_projectordb.py	2016-06-04 08:15:27 +0000
@@ -284,3 +284,16 @@
         self.assertEqual(str(source),
                          '<ProjectorSource(id="1", code="11", text="First RGB source", projector_id="1")>',
                          'ProjectorSource.__repr__)_ should have returned a proper representation string')
+
+    def test_get_projector_by_id_none(self):
+        """
+        Test get_projector_by_id returns None if no db entry
+        """
+        # GIVEN: Test object and data
+        projector = self.projector
+
+        # WHEN: DB search for entry not saved
+        results = projector.get_projector_by_id(dbid=123134556409824506)
+
+        # THEN: Verify return was None
+        self.assertEqual(results, None, 'Returned results should have equaled None')

=== modified file 'tests/functional/openlp_plugins/bibles/test_lib.py'
--- tests/functional/openlp_plugins/bibles/test_lib.py	2016-06-01 23:14:58 +0000
+++ tests/functional/openlp_plugins/bibles/test_lib.py	2016-06-04 08:15:27 +0000
@@ -43,6 +43,7 @@
         separators = {'sep_r': '\\s*(?:e)\\s*', 'sep_e_default': 'end', 'sep_v_display': 'w', 'sep_l_display': 'r',
                       'sep_v_default': ':|v|V|verse|verses', 'sep_l': '\\s*(?:r)\\s*', 'sep_l_default': ',|and',
                       'sep_e': '\\s*(?:t)\\s*', 'sep_v': '\\s*(?:w)\\s*', 'sep_r_display': 'e', 'sep_r_default': '-|to'}
+
         def _update_side_effect():
             """
             Update the references after mocking out the method

=== modified file 'tests/functional/openlp_plugins/songs/test_opsproimport.py'
--- tests/functional/openlp_plugins/songs/test_opsproimport.py	2016-06-01 23:14:58 +0000
+++ tests/functional/openlp_plugins/songs/test_opsproimport.py	2016-06-04 08:15:27 +0000
@@ -171,4 +171,3 @@
         result_data = json.loads(result_file.read().decode())
         self.assertListEqual(importer.verses, _get_item(result_data, 'verses'))
         self.assertListEqual(importer.verse_order_list_generated, _get_item(result_data, 'verse_order_list'))
-


Follow ups