Ken Roberts has proposed merging lp:~alisonken1/openlp/strings-templates into lp:openlp.
Commit message: String conversions with templates part 1 Requested reviews: Raoul Snyman (raoul-snyman) Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~alisonken1/openlp/strings-templates/+merge/297641 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 - Convert renderer to use Template() - Added test for renderer css -------------------------------- lp:~alisonken1/openlp/strings-templates (revision 2679) [SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1621/ [SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1532/ [SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1470/ [SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1241/ [SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/831/ [SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/899/ [SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/767/ -- 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-16 15:32:40 +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-16 15:32:40 +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-16 15:32:40 +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-16 15:32:40 +0000 @@ -22,6 +22,7 @@ import re +from string import Template from PyQt5 import QtGui, QtCore, QtWebKitWidgets from openlp.core.common import Registry, RegistryProperties, OpenLPMixin, RegistryMixin, Settings @@ -370,8 +371,8 @@ 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 - html = """<!DOCTYPE html><html><head><script> + # TODO: Tested at home + html = Template("""<!DOCTYPE html><html><head><script> function show_text(newtext) { var main = document.getElementById('main'); main.innerHTML = newtext; @@ -380,12 +381,16 @@ // 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)) - self.web.setHtml(html) + </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>""") + self.web.setHtml(html.substitute(format_css=build_lyrics_format_css(theme_data, + self.page_width, + self.page_height), + outline_css=build_lyrics_outline_css(theme_data))) self.empty_height = self.web_frame.contentsSize().height() def _paginate_slide(self, lines, line_end): === 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-16 15:32:40 +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-16 15:32:40 +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:b...@openlp.org') === modified file 'openlp/core/ui/firsttimeform.py' --- openlp/core/ui/firsttimeform.py 2016-06-09 02:57:21 +0000 +++ openlp/core/ui/firsttimeform.py 2016-06-16 15:32:40 +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,8 +564,13 @@ self.progress_bar.setValue(self.progress_bar.maximum()) if self.has_run_wizard: text = translate('OpenLP.FirstTimeWizard', +<<<<<<< TREE 'Download complete. Click the {button} button to return to OpenLP.' ).format(button=clean_button_text(self.buttonText(QtWidgets.QWizard.FinishButton))) +======= + 'Download complete. Click the {text} button to return to OpenLP.' + ).format(text=clean_button_text(self.buttonText(QtWidgets.QWizard.FinishButton))) +>>>>>>> MERGE-SOURCE self.progress_label.setText(text) else: text = translate('OpenLP.FirstTimeWizard', @@ -632,7 +637,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 +650,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 +663,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-16 15:32:40 +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-16 15:32:40 +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-16 15:32:40 +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-16 15:32:40 +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_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-16 15:32:40 +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-16 15:32:40 +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-16 15:32:40 +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_core_lib/test_renderer.py' --- tests/functional/openlp_core_lib/test_renderer.py 2016-05-31 21:40:13 +0000 +++ tests/functional/openlp_core_lib/test_renderer.py 2016-06-16 15:32:40 +0000 @@ -29,6 +29,7 @@ from openlp.core.common import Registry from openlp.core.lib import Renderer, ScreenList, ServiceItem, FormattingTags from openlp.core.lib.renderer import words_split, get_start_tags +from openlp.core.lib.theme import ThemeXML from tests.functional import MagicMock, patch @@ -39,6 +40,24 @@ } +# WARNING: Leave formatting alone - this is how it's returned in renderer.py +CSS_TEST_ONE = """<!DOCTYPE html><html><head><script> + 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; FORMAT CSS; OUTLINE CSS; } + </style></head> + <body><div id="main"></div></body></html>'""" + + class TestRenderer(TestCase): def setUp(self): @@ -159,3 +178,28 @@ # THEN: The blanks have been removed. self.assertListEqual(result_words, expected_words) + + @patch('openlp.core.lib.renderer.QtWebKitWidgets.QWebView') + @patch('openlp.core.lib.renderer.build_lyrics_format_css') + @patch('openlp.core.lib.renderer.build_lyrics_outline_css') + def test_set_text_rectangle(self, mock_outline_css, mock_lyrics_css, mock_webview): + """ + Test set_text_rectangle returns a proper html string + """ + # GIVEN: test object and data + mock_lyrics_css.return_value = ' FORMAT CSS; ' + mock_outline_css.return_value = ' OUTLINE CSS; ' + theme_data = ThemeXML() + theme_data.font_main_name = 'Arial' + theme_data.font_main_size = 20 + theme_data.font_main_color = '#FFFFFF' + theme_data.font_main_outline_color = '#FFFFFF' + main = QtCore.QRect(10, 10, 1280, 900) + foot = QtCore.QRect(10, 1000, 1260, 24) + renderer = Renderer() + + # WHEN: Calling method + renderer._set_text_rectangle(theme_data=theme_data, rect_main=main, rect_footer=foot) + + # THEN: QtWebKitWidgets should be called with the proper string + mock_webview.setHtml.called_with(CSS_TEST_ONE, 'Should be the same')
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp