[Openlp-core] [Merge] lp:~trb143/openlp/cleanups02182 into lp:openlp

2018-03-09 Thread Tim Bentley 
The proposal to merge lp:~trb143/openlp/cleanups02182 into lp:openlp has been 
updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~trb143/openlp/cleanups02182/+merge/339422
-- 
Your team OpenLP Core is requested to review the proposed merge of 
lp:~trb143/openlp/cleanups02182 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:~trb143/openlp/cleanups02182 into lp:openlp

2018-03-09 Thread Tim Bentley 
Tim Bentley has proposed merging lp:~trb143/openlp/cleanups02182 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1016078 in OpenLP: "Can't clear preview pane"
  https://bugs.launchpad.net/openlp/+bug/1016078
  Bug #1449477 in OpenLP: "lyric title via api contoller text"
  https://bugs.launchpad.net/openlp/+bug/1449477
  Bug #1512689 in OpenLP: "automatically created slides"
  https://bugs.launchpad.net/openlp/+bug/1512689
  Bug #1544260 in OpenLP: "Moving Song in Service Manager, Hover Text says 
"Service Manager""
  https://bugs.launchpad.net/openlp/+bug/1544260
  Bug #1651823 in OpenLP: "Make it possible to clone a Custom Slide"
  https://bugs.launchpad.net/openlp/+bug/1651823

For more details, see:
https://code.launchpad.net/~trb143/openlp/cleanups02182/+merge/341268

Add some small items from wish lists
Various cleanups.


lp:~trb143/openlp/cleanups02182 (revision 2832)
https://ci.openlp.io/job/Branch-01-Pull/2469/  [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2370/  [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/157/   [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/79/   [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/73/[SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1541/[SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1354/[SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/287/ [RUNNING]

-- 
Your team OpenLP Core is requested to review the proposed merge of 
lp:~trb143/openlp/cleanups02182 into lp:openlp.
=== modified file 'openlp/core/api/endpoint/controller.py'
--- openlp/core/api/endpoint/controller.py	2017-12-29 09:15:48 +
+++ openlp/core/api/endpoint/controller.py	2018-03-09 21:16:28 +
@@ -91,6 +91,7 @@
 item['text'] = str(frame['title'])
 item['html'] = str(frame['title'])
 item['selected'] = (live_controller.selected_row == index)
+item['title'] = current_item.title
 data.append(item)
 json_data = {'results': {'slides': data}}
 if current_item:
@@ -117,12 +118,11 @@
 return {'results': {'success': True}}
 
 
-@controller_endpoint.route('{action:next|previous}')
+@controller_endpoint.route('{controller}/{action:next|previous}')
 @requires_auth
 def controller_direction(request, controller, action):
 """
 Handles requests for setting service items in the slide controller
-11
 :param request: The http request object.
 :param controller: the controller slides forward or backward.
 :param action: the controller slides forward or backward.
@@ -137,7 +137,7 @@
 def controller_direction_api(request, controller, action):
 """
 Handles requests for setting service items in the slide controller
-11
+
 :param request: The http request object.
 :param controller: the controller slides forward or backward.
 :param action: the controller slides forward or backward.

=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py	2018-02-11 21:52:04 +
+++ openlp/core/common/__init__.py	2018-03-09 21:16:28 +
@@ -62,7 +62,7 @@
 """
 # Get the local IPv4 active address(es) that are NOT localhost (lo or '127.0.0.1')
 log.debug('Getting local IPv4 interface(es) information')
-MY_IP4 = {}
+my_ip4 = {}
 for iface in QNetworkInterface.allInterfaces():
 if not iface.isValid() or not (iface.flags() & (QNetworkInterface.IsUp | QNetworkInterface.IsRunning)):
 continue
@@ -70,25 +70,25 @@
 ip = address.ip()
 # NOTE: Next line will skip if interface is localhost - keep for now until we decide about it later
 # if (ip.protocol() == QAbstractSocket.IPv4Protocol) and (ip != QHostAddress.LocalHost):
-if (ip.protocol() == QAbstractSocket.IPv4Protocol):
-MY_IP4[iface.name()] = {'ip': ip.toString(),
+if ip.protocol() == QAbstractSocket.IPv4Protocol:
+my_ip4[iface.name()] = {'ip': ip.toString(),
 'broadcast': address.broadcast().toString(),
 'netmask': address.netmask().toString(),
 'prefix': address.prefixLength(),
 'localnet': QHostAddress(address.netmask().toIPv4Address() &
-ip.toIPv4Address()).toString()
+ ip.toIPv4Address()).toString()
 }
 log.debug('Adding {iface} to active list'.format(iface=iface.name()))
-if len(MY_IP4) == 1:
-if 'lo' in MY_IP4:
+i

[Openlp-core] [Bug 1751628] Re: portabe flag is marked as not implemented

2018-03-09 Thread Tim Bentley 
The running from a USB device may not be implemented the rest of the
portable code should work as it available on Openlp.org

** Changed in: openlp
   Importance: Undecided => Low

** Changed in: openlp
   Status: New => Confirmed

-- 
You received this bug notification because you are a member of OpenLP
Core, which is subscribed to OpenLP.
https://bugs.launchpad.net/bugs/1751628

Title:
  portabe flag is marked as not implemented

Status in OpenLP:
  Confirmed

Bug description:
  OpenLP has a portable flag that runs OpenLP with a data/config
  directory relative to the path where it's stored. I think it is built
  in a windows specific way (where the stuff is then stored) but it
  seems to work flawlessly on my linux machine.

  However the output of -h says that it's not implemented. I am pretty
  sure that the documentation is wrong, but someone else might know more
  about that.

To manage notifications about this bug go to:
https://bugs.launchpad.net/openlp/+bug/1751628/+subscriptions

___
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] [Bug 1751626] Re: -d flag is used in a very confusing way

2018-03-09 Thread Tim Bentley 
This works fine and does provide so real functionality.

without the -d file shows the version from the .version file which is
controlled from bzr code and matches the main code version.

with the -d uses the version from bzr and changes the version
information using bzr tags.

This is useful for when testing automated upgrades.

Unless you are a dev or running code versions you will not know about
this so it is not an issue.

** Changed in: openlp
   Status: New => Won't Fix

-- 
You received this bug notification because you are a member of OpenLP
Core, which is subscribed to OpenLP.
https://bugs.launchpad.net/bugs/1751626

Title:
  -d flag is used in a very confusing way

Status in OpenLP:
  Won't Fix

Bug description:
  in the startup code the parameters accepted include --dev-version
  (-d).

  What's irritating is that it's not actually used there. The values are
  also not actually passed along somewhere else.

  However the about dialog still correctly displays the bzr revision if
  started with -d.

  So a little searching showed the following. The argument is defined in
  openlp.core.app.parse_options but only used in
  openlp.core.version.get_version

  Since the dev_version is not stored somewhere or passed to some place,
  get_version actually checks sys.argv searching for "-d" or "--dev-
  version"

  This is not very nice to people reading the code. It very much looks
  like the argument definition is not actually used.

  My request:
  * either completely remove the functionality
  * or properly use the parsed value, so it's clear it actually does something.

  also having the bzr check code in openlp.core.version.get_version is
  probably not the best idea. Especially since the code includes a
  comment that it's a duplicate of code in setup.py

  Further rants:
  in setup.py the code is not actually exactly the same, there are 
modifications and there is a comment that it is a duplicate of the no longer 
existing openlp.core.common.checkversions.py 

  This illustrates that comments are bad for the reason that they are
  often not updated when the code changes. This can lead to comments
  confusing more than helping.

  
  In short:
  Remove the flag, remove the code from get_version and only have it in setup.py
  The flag is only useful for people running from bzr so devs, which should 
know what they are running and even if they don't they should know how to get 
that information without reverting to the about tab.

To manage notifications about this bug go to:
https://bugs.launchpad.net/openlp/+bug/1751626/+subscriptions

___
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

Re: [Openlp-core] [Merge] lp:~trb143/openlp/cleanups02182 into lp:openlp

2018-03-09 Thread Raoul Snyman 
Review: Needs Fixing

Been coding a lot in C recently? ;-)

Diff comments:

> 
> === modified file 'openlp/core/ui/slidecontroller.py'
> --- openlp/core/ui/slidecontroller.py 2018-02-03 11:32:49 +
> +++ openlp/core/ui/slidecontroller.py 2018-03-09 21:16:28 +
> @@ -1381,6 +1387,14 @@
>  if new_item:
>  self.add_service_item(new_item)
>  
> +def on_clear(self):
> +"""
> +Clear the preview bar.
> +"""
> +self.preview_widget.clear_list()
> +self.toolbar.set_widget_visible("editSong", False)

Single quotes

> +self.toolbar.set_widget_visible("clear", False)

Single quotes

> +
>  def on_preview_add_to_service(self):
>  """
>  From the preview display request the Item to be added to service
> 
> === modified file 'openlp/plugins/songs/lib/mediaitem.py'
> --- openlp/plugins/songs/lib/mediaitem.py 2018-01-19 21:31:36 +
> +++ openlp/plugins/songs/lib/mediaitem.py 2018-03-09 21:16:28 +
> @@ -572,10 +572,19 @@
>  service_item.add_capability(ItemCapabilities.OnLoadUpdate)
>  service_item.add_capability(ItemCapabilities.AddIfNewItem)
>  service_item.add_capability(ItemCapabilities.CanSoftBreak)
> +service_item.add_capability(ItemCapabilities.HasMetaData)
>  song = self.plugin.manager.get_object(Song, item_id)
>  service_item.theme = song.theme_name
>  service_item.edit_id = item_id
>  verse_list = SongXML().get_verses(song.lyrics)
> +if Settings().value('songs/add songbook slide') and 
> song.songbook_entries:
> +first_slide = "\n"

Single quotes

> +for songbook_entry in song.songbook_entries:
> +first_slide = first_slide + 
> "{book}/{num}/{pub}\n\n".format(book=songbook_entry.songbook.name,

Single quotes

> +
> num=songbook_entry.entry,
> +
> pub=songbook_entry.songbook.publisher)
> +
> +service_item.add_from_text(first_slide, "O1")

Single quotes

>  # no verse list or only 1 space (in error)
>  verse_tags_translated = False
>  if VerseType.from_translated_string(str(verse_list[0][0]['type'])) 
> is not None:
> @@ -622,6 +631,9 @@
>  if song.media_files:
>  service_item.add_capability(ItemCapabilities.HasBackgroundAudio)
>  service_item.background_audio = [m.file_path for m in 
> song.media_files]
> +item.metadata.append("{label}: {media}".

Single quotes

> + 
> format(label=translate('SongsPlugin.MediaItem', 'Media'),
> +media=service_item.background_audio))
>  return True
>  
>  def generate_footer(self, item, song):
> @@ -685,6 +697,23 @@
>  if Settings().value('core/ccli number'):
>  item.raw_footer.append(translate('SongsPlugin.MediaItem',
>   'CCLI License: ') + 
> Settings().value('core/ccli number'))
> +item.metadata.append("{label}: 
> {title}".format(label=translate('SongsPlugin.MediaItem', 'Title'),

Single quotes

> +
> title=song.title))
> +if song.alternate_title:
> +item.metadata.append("{label}: {title}".

Single quotes

> + 
> format(label=translate('SongsPlugin.MediaItem', 'Alt Title'),
> +title=song.alternate_title))
> +if song.songbook_entries:
> +for songbook_entry in song.songbook_entries:
> +item.metadata.append("{label}: {book}/{num}/{pub}".

Single quotes

> + 
> format(label=translate('SongsPlugin.MediaItem', 'Songbook'),
> +
> book=songbook_entry.songbook.name,
> +num=songbook_entry.entry,
> +
> pub=songbook_entry.songbook.publisher))
> +if song.topics:
> +for topics in song.topics:
> +item.metadata.append("{label}: {topic}".

Single quotes

> + 
> format(label=translate('SongsPlugin.MediaItem', 'Topic'), topic=topics.name))
>  return authors_all
>  
>  def service_load(self, item):
> 
> === modified file 
> 'tests/functional/openlp_core/api/endpoint/test_controller.py'
> --- tests/functional/openlp_core/api/endpoint/test_controller.py  
> 2018-02-03 14:41:18 +
> +++ tests/functional/openlp_core/api/endpoint/test_controller.py  
> 2018-03-09 21:16:28 +
> @@ -52,3 +52,25 @@
>  results = ret['results']
>  assert isinstance(results['item'], MagicMock)
>  assert len(results['slides']) == 0
> +
> +def test_co

[Openlp-core] [Merge] lp:~trb143/openlp/cleanups02182 into lp:openlp

2018-03-09 Thread Tim Bentley 
The proposal to merge lp:~trb143/openlp/cleanups02182 into lp:openlp has been 
updated.

Status: Needs review => Superseded

For more details, see:
https://code.launchpad.net/~trb143/openlp/cleanups02182/+merge/341268
-- 
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:~trb143/openlp/cleanups02182 into lp:openlp

2018-03-09 Thread Tim Bentley 
Tim Bentley has proposed merging lp:~trb143/openlp/cleanups02182 into lp:openlp.

Requested reviews:
  Raoul Snyman (raoul-snyman)
Related bugs:
  Bug #1016078 in OpenLP: "Can't clear preview pane"
  https://bugs.launchpad.net/openlp/+bug/1016078
  Bug #1449477 in OpenLP: "lyric title via api contoller text"
  https://bugs.launchpad.net/openlp/+bug/1449477
  Bug #1512689 in OpenLP: "automatically created slides"
  https://bugs.launchpad.net/openlp/+bug/1512689
  Bug #1544260 in OpenLP: "Moving Song in Service Manager, Hover Text says 
"Service Manager""
  https://bugs.launchpad.net/openlp/+bug/1544260
  Bug #1651823 in OpenLP: "Make it possible to clone a Custom Slide"
  https://bugs.launchpad.net/openlp/+bug/1651823

For more details, see:
https://code.launchpad.net/~trb143/openlp/cleanups02182/+merge/341270

Add some small items from wish lists
Various cleanups.


lp:~trb143/openlp/cleanups02182 (revision 2833)
https://ci.openlp.io/job/Branch-01-Pull/2470/  [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2371/  [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/158/   [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/80/   [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/74/[SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1542/[SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1355/[SUCCESS]
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/api/endpoint/controller.py'
--- openlp/core/api/endpoint/controller.py	2017-12-29 09:15:48 +
+++ openlp/core/api/endpoint/controller.py	2018-03-09 22:06:33 +
@@ -91,6 +91,7 @@
 item['text'] = str(frame['title'])
 item['html'] = str(frame['title'])
 item['selected'] = (live_controller.selected_row == index)
+item['title'] = current_item.title
 data.append(item)
 json_data = {'results': {'slides': data}}
 if current_item:
@@ -117,12 +118,11 @@
 return {'results': {'success': True}}
 
 
-@controller_endpoint.route('{action:next|previous}')
+@controller_endpoint.route('{controller}/{action:next|previous}')
 @requires_auth
 def controller_direction(request, controller, action):
 """
 Handles requests for setting service items in the slide controller
-11
 :param request: The http request object.
 :param controller: the controller slides forward or backward.
 :param action: the controller slides forward or backward.
@@ -137,7 +137,7 @@
 def controller_direction_api(request, controller, action):
 """
 Handles requests for setting service items in the slide controller
-11
+
 :param request: The http request object.
 :param controller: the controller slides forward or backward.
 :param action: the controller slides forward or backward.

=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py	2018-02-11 21:52:04 +
+++ openlp/core/common/__init__.py	2018-03-09 22:06:33 +
@@ -62,7 +62,7 @@
 """
 # Get the local IPv4 active address(es) that are NOT localhost (lo or '127.0.0.1')
 log.debug('Getting local IPv4 interface(es) information')
-MY_IP4 = {}
+my_ip4 = {}
 for iface in QNetworkInterface.allInterfaces():
 if not iface.isValid() or not (iface.flags() & (QNetworkInterface.IsUp | QNetworkInterface.IsRunning)):
 continue
@@ -70,25 +70,25 @@
 ip = address.ip()
 # NOTE: Next line will skip if interface is localhost - keep for now until we decide about it later
 # if (ip.protocol() == QAbstractSocket.IPv4Protocol) and (ip != QHostAddress.LocalHost):
-if (ip.protocol() == QAbstractSocket.IPv4Protocol):
-MY_IP4[iface.name()] = {'ip': ip.toString(),
+if ip.protocol() == QAbstractSocket.IPv4Protocol:
+my_ip4[iface.name()] = {'ip': ip.toString(),
 'broadcast': address.broadcast().toString(),
 'netmask': address.netmask().toString(),
 'prefix': address.prefixLength(),
 'localnet': QHostAddress(address.netmask().toIPv4Address() &
-ip.toIPv4Address()).toString()
+ ip.toIPv4Address()).toString()
 }
 log.debug('Adding {iface} to active list'.format(iface=iface.name()))
-if len(MY_IP4) == 1:
-if 'lo' in MY_IP4:
+if len(my_ip4) == 1:
+if 'lo' in my_ip4:
 # No active interfaces - so leave localhost in there
 log.warning(