Re: [Openlp-core] [Merge] lp:~john+ubuntu-g/openlp/singingthefaith into lp:openlp

[Phill]

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

[Raoul Snyman]

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]

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
+}, {

[Openlp-core] macOS Test Results: Passed

[Raoul Snyman]

macOS tests passed!
-- 
https://code.launchpad.net/~raoul-snyman/openlp/zeroconf/+merge/369632
Your team OpenLP Core is subscribed to branch 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] Linting: Passed

[Raoul Snyman]

Linting passed!
-- 
https://code.launchpad.net/~raoul-snyman/openlp/zeroconf/+merge/369632
Your team OpenLP Core is subscribed to branch 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] Linux Test Results: Passed

[Raoul Snyman]

Linux tests passed!
-- 
https://code.launchpad.net/~raoul-snyman/openlp/zeroconf/+merge/369632
Your team OpenLP Core is subscribed to branch 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:~raoul-snyman/openlp/zeroconf into lp:openlp

[Raoul Snyman]

Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/zeroconf into 
lp:openlp.

Commit message:
Add Zeroconf services to OpenLP so that external devices can find OpenLP on the 
network.

Requested reviews:
  Tomas Groth (tomasgroth)

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/zeroconf/+merge/369632

Add Zeroconf services to OpenLP so that external devices can find OpenLP on the 
network.
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/api/tab.py'
--- openlp/core/api/tab.py	2019-04-13 13:00:22 +
+++ openlp/core/api/tab.py	2019-07-03 06:34:48 +
@@ -24,7 +24,7 @@
 """
 from PyQt5 import QtCore, QtGui, QtWidgets
 
-from openlp.core.common import get_local_ip4
+from openlp.core.common import get_network_interfaces
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -194,8 +194,7 @@
 http_url_temp = http_url + 'main'
 self.live_url.setText('{url}'.format(url=http_url_temp))
 
-@staticmethod
-def get_ip_address(ip_address):
+def get_ip_address(self, ip_address):
 """
 returns the IP address in dependency of the passed address
 ip_address == 0.0.0.0: return the IP address of the first valid interface
@@ -203,9 +202,8 @@
 """
 if ip_address == ZERO_URL:
 # In case we have more than one interface
-ifaces = get_local_ip4()
-for key in iter(ifaces):
-ip_address = ifaces.get(key)['ip']
+for _, interface in get_network_interfaces().items():
+ip_address = interface['ip']
 # We only want the first interface returned
 break
 return ip_address

=== added file 'openlp/core/api/zeroconf.py'
--- openlp/core/api/zeroconf.py	1970-01-01 00:00:00 +
+++ openlp/core/api/zeroconf.py	2019-07-03 06:34:48 +
@@ -0,0 +1,99 @@
+# -*- 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, either version 3 of the License, or  #
+# (at your option) any later version.#
+##
+# 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, see . #
+##
+"""
+The :mod:`~openlp.core.api.zeroconf` module runs a Zerconf server so that OpenLP can advertise the
+RESTful API for devices on the network to discover.
+"""
+import socket
+from time import sleep
+
+from zeroconf import ServiceInfo, Zeroconf
+
+from openlp.core.common import get_network_interfaces
+from openlp.core.common.registry import Registry
+from openlp.core.common.settings import Settings
+from openlp.core.threading import ThreadWorker, run_thread
+
+
+class ZeroconfWorker(ThreadWorker):
+"""
+This thread worker runs a Zeroconf service
+"""
+address = None
+http_port = 4316
+ws_port = 4317
+_can_run = False
+
+def __init__(self, ip_address, http_port=4316, ws_port=4317):
+"""
+Create the worker for the Zeroconf service
+"""
+super().__init__()
+self.address = socket.inet_aton(ip_address)
+self.http_port = http_port
+self.ws_port = ws_port
+
+def can_run(self):
+"""
+Check if the worker can continue to run. This is mostly so that we can override this method
+and test the class.
+"""
+return self._can_run
+
+def start(self):
+"""
+Start the service
+"""
+http_info = ServiceInfo('_http._tcp.local.', 'OpenLP._http._tcp.local.',
+address=self.address, port=self.http_port, properties={})
+ws_info = ServiceInfo('_ws._tcp.local.', 'OpenLP._ws._tcp.local.',
+  

[Openlp-core] [Merge] lp:~raoul-snyman/openlp/zeroconf into lp:openlp

[Raoul Snyman]

The proposal to merge lp:~raoul-snyman/openlp/zeroconf into lp:openlp has been 
updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/zeroconf/+merge/369623
-- 
Your team OpenLP Core is subscribed to branch 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:~raoul-snyman/openlp/fix-presentations-cleanup-bug into lp:openlp

[noreply]

The proposal to merge lp:~raoul-snyman/openlp/fix-presentations-cleanup-bug 
into lp:openlp has been updated.

Status: Approved => Merged

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/fix-presentations-cleanup-bug/+merge/369624
-- 
Your team OpenLP Core is subscribed to branch 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:~raoul-snyman/openlp/fix-presentations-cleanup-bug into lp:openlp

[Raoul Snyman]

The proposal to merge lp:~raoul-snyman/openlp/fix-presentations-cleanup-bug 
into lp:openlp has been updated.

Status: Needs review => Approved

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/fix-presentations-cleanup-bug/+merge/369624
-- 
Your team OpenLP Core is subscribed to branch 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