Re: [Openlp-core] [Merge] lp:~john+ubuntu-g/openlp/singingthefaith into lp:openlp
Review: Needs Fixing
Please change your string formatting to use the 'new' style with the format
function. ( https://pyformat.info/ ) also string formatting is preferred over
concatenation (i.e, "part1" + var + "part2")
single quotes for strings, not double quotes
do_import_file is very long can this be split in to smaller methods?
Diff comments:
>
> === added file 'openlp/plugins/songs/lib/importers/singingthefaith.py'
> --- openlp/plugins/songs/lib/importers/singingthefaith.py 1970-01-01
> 00:00:00 +
> +++ openlp/plugins/songs/lib/importers/singingthefaith.py 2019-06-30
> 19:19:46 +
> @@ -0,0 +1,347 @@
> +# -*- coding: utf-8 -*-
> +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4
> softtabstop=4
> +
> +###
> +# OpenLP - Open Source Lyrics Projection
> #
> +#
> --- #
> +# Copyright (c) 2008-2019 OpenLP Developers
> #
> +#
> --- #
> +# This program is free software; you can redistribute it and/or modify it
> #
> +# under the terms of the GNU General Public License as published by the Free
> #
> +# Software Foundation; version 2 of the License.
> #
> +#
> #
> +# This program is distributed in the hope that it will be useful, but
> WITHOUT #
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> #
> +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> #
> +# more details.
> #
> +#
> #
> +# You should have received a copy of the GNU General Public License along
> #
> +# with this program; if not, write to the Free Software Foundation, Inc., 59
> #
> +# Temple Place, Suite 330, Boston, MA 02111-1307 USA
> #
> +###
> +"""
> +The :mod:`singingthefaith` module provides the functionality for importing
> songs which are
> +exported from Singing The Faith - an Authorised songbook for the Methodist
> Church of
> +Great Britain."""
> +
> +import logging
> +import re
> +from pathlib import Path
> +
> +from openlp.core.common.i18n import translate
> +from openlp.plugins.songs.lib.importers.songimport import SongImport
> +
> +log = logging.getLogger(__name__)
> +
> +
> +class SingingTheFaithImport(SongImport):
> +"""
> +Import songs exported from SingingTheFaith
> +"""
> +
> +hints_available = False
> +checks_needed = True
> +hintline = {}
can we have some consistency here and make these hint_line
> +hintfile_version = '0'
and hint_file_version
> +hint_verseOrder = ''
also please stick to 'snake_case' variable names like hint_verse_order
> +hint_songtitle = ''
hint_song_title
> +hint_comments = ''
> +hint_ignoreIndent = False
hint_ignore_indent
> +
> +def do_import(self):
> +"""
> +Receive a single file or a list of files to import.
> +"""
> +if not isinstance(self.import_source, list):
> +return
> +self.import_wizard.progress_bar.setMaximum(len(self.import_source))
> +for file_path in self.import_source:
> +if self.stop_import_flag:
> +return
> +with file_path.open('rt', encoding='cp1251') as song_file:
> +self.do_import_file(song_file)
> +
> +def do_import_file(self, file):
> +"""
> +Process the SingingTheFaith file - pass in a file-like object, not a
> file path.
> +"""
> +singingTheFaithVersion = 1
singing_the_faith_version
> +self.set_defaults()
> +# Setup variables
> +line_number = 0
> +old_indent = 0
> +# The chorus indent is how many spaces the chorus is indented - it
> might be 6,
> +# but we test for >= and I do not know how consistent to formatting
> of the
> +# exported songs is.
> +chorus_indent = 5
> +song_title = 'STF000 -'
> +song_number = '0'
> +ccli = '0'
> +current_verse = ''
> +current_verse_type = 'v'
> +current_verse_number = 1
> +# Potentially we could try to track current chorus number to
> automatically handle
> +# more than 1 chorus, currently unused.
> +# current_chorus_number = 1
> +has_chorus = False
> +chorus_written = False
> +auto_verse_order_ok = False
> +copyright = ''
> +# the check_flag is prepended to the title, removed if the import
> should be OK
>
[Openlp-core] Linux Test Results: Failed
Linux tests failed, please see https://ci.openlp.io/job/MP-02-Linux_Tests/206/ for more details -- https://code.launchpad.net/~phill-ridout/openlp/fixes-III/+merge/369653 Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/fixes-III into lp:openlp. ___ 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
[Openlp-core] [Merge] lp:~phill-ridout/openlp/fixes-III into lp:openlp
Phill has proposed merging lp:~phill-ridout/openlp/fixes-III into lp:openlp.
Commit message:
Minor fixes and changes
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/fixes-III/+merge/369653
--
Your team OpenLP Core is requested to review the proposed merge of
lp:~phill-ridout/openlp/fixes-III into lp:openlp.
=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py 2019-06-05 04:53:18 +
+++ openlp/core/common/__init__.py 2019-07-03 13:29:54 +
@@ -45,7 +45,7 @@
FIRST_CAMEL_REGEX = re.compile('(.)([A-Z][a-z]+)')
SECOND_CAMEL_REGEX = re.compile('([a-z0-9])([A-Z])')
CONTROL_CHARS = re.compile(r'[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]')
-INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]')
+INVALID_FILE_CHARS = re.compile(r'[\\/:*?"<>|+\[\]%]')
IMAGES_FILTER = None
REPLACMENT_CHARS_MAP = str.maketrans({'\u2018': '\'', '\u2019': '\'', '\u201c': '"', '\u201d': '"', '\u2026': '...',
'\u2013': '-', '\u2014': '-', '\v': '\n\n', '\f': '\n\n'})
@@ -112,21 +112,21 @@
logger.error(log_string)
-def extension_loader(glob_pattern, excluded_files=[]):
+def extension_loader(glob_pattern, excluded_files=None):
"""
A utility function to find and load OpenLP extensions, such as plugins, presentation and media controllers and
importers.
:param str glob_pattern: A glob pattern used to find the extension(s) to be imported. Should be relative to the
application directory. i.e. plugins/*/*plugin.py
-:param list[str] excluded_files: A list of file names to exclude that the glob pattern may find.
+:param list[str] | None excluded_files: A list of file names to exclude that the glob pattern may find.
:rtype: None
"""
from openlp.core.common.applocation import AppLocation
app_dir = AppLocation.get_directory(AppLocation.AppDir)
for extension_path in app_dir.glob(glob_pattern):
extension_path = extension_path.relative_to(app_dir)
-if extension_path.name in excluded_files:
+if extension_path.name in (excluded_files or []):
continue
log.debug('Attempting to import %s', extension_path)
module_name = path_to_module(extension_path)
=== modified file 'openlp/core/common/i18n.py'
--- openlp/core/common/i18n.py 2019-06-28 18:09:25 +
+++ openlp/core/common/i18n.py 2019-07-03 13:29:54 +
@@ -338,8 +338,8 @@
Override the default object creation method to return a single instance.
"""
if not cls.__instance__:
-cls.__instance__ = object.__new__(cls)
-cls.load(cls)
+cls.__instance__ = super().__new__(cls)
+cls.__instance__.load()
return cls.__instance__
def load(self):
@@ -503,7 +503,7 @@
"""
return local_time.strftime(match.group())
-return re.sub(r'\%[a-zA-Z]', match_formatting, text)
+return re.sub(r'%[a-zA-Z]', match_formatting, text)
def get_locale_key(string, numeric=False):
=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py 2019-05-22 20:46:51 +
+++ openlp/core/lib/db.py 2019-07-03 13:29:54 +
@@ -265,7 +265,7 @@
"""
if not database_exists(url):
log.warning("Database {db} doesn't exist - skipping upgrade checks".format(db=url))
-return (0, 0)
+return 0, 0
log.debug('Checking upgrades for DB {db}'.format(db=url))
=== modified file 'openlp/core/lib/formattingtags.py'
--- openlp/core/lib/formattingtags.py 2019-04-13 13:00:22 +
+++ openlp/core/lib/formattingtags.py 2019-07-03 13:29:54 +
@@ -59,97 +59,98 @@
"""
temporary_tags = [tag for tag in FormattingTags.html_expands if tag.get('temporary')]
FormattingTags.html_expands = []
-base_tags = []
+base_tags = [
+{
+'desc': translate('OpenLP.FormattingTags', 'Red'),
+'start tag': '{r}',
+'start html': '',
+'end tag': '{/r}', 'end html': '', 'protected': True,
+'temporary': False
+}, {
+'desc': translate('OpenLP.FormattingTags', 'Black'),
+'start tag': '{b}',
+'start html': '',
+'end tag': '{/b}', 'end html': '', 'protected': True,
+'temporary': False
+}, {
+'desc': translate('OpenLP.FormattingTags', 'Blue'),
+'start tag': '{bl}',
+'start html': '',
+'end tag': '{/bl}', 'end html': '', 'protected': True,
+'temporary': False
+}, {
+'desc': translate('OpenLP.FormattingTags', 'Yellow'),
+'start tag': '{y}',
+'start html': '',
+'end tag': '{/y}', 'end html': '', 'protected': True,
+'temporary': False
+}, {
