openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Tim Bentley has proposed merging lp:~trb143/openlp/bug-1202677 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1202677 in OpenLP: "If optional split is not alone on a line, OpenLP crashes"
  https://bugs.launchpad.net/openlp/+bug/1202677

For more details, see:
https://code.launchpad.net/~trb143/openlp/bug-1202677/+merge/241866

Old ones are the best.

Add this to your merge proposal:
--------------------------------
lp:~trb143/openlp/bug-1202677 (revision 2421)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/776/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/712/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/658/
[FAILURE] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/595/

Wine has been at the booze again!
-- 
https://code.launchpad.net/~trb143/openlp/bug-1202677/+merge/241866
Your team OpenLP Core is requested to review the proposed merge of lp:~trb143/openlp/bug-1202677 into lp:openlp.

=== modified file 'openlp/core/lib/renderer.py'
--- openlp/core/lib/renderer.py	2014-07-02 18:13:23 +0000
+++ openlp/core/lib/renderer.py	2014-11-15 09:52:34 +0000
@@ -250,7 +250,13 @@
                 # Remove two or more option slide breaks next to each other (causing infinite loop).
                 while '\n[---]\n[---]\n' in text:
                     text = text.replace('\n[---]\n[---]\n', '\n[---]\n')
-                while True:
+                while ' [---]' in text:
+                    text = text.replace(' [---]', '[---]')
+                while '[---] ' in text:
+                    text = text.replace('[---] ', '[---]')
+                count = 0
+                # only loop 5 times as there will never be more than 5 logical splits on a slide.
+                while True and count < 5:
                     slides = text.split('\n[---]\n', 2)
                     # If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last
                     # for now).
@@ -296,6 +302,7 @@
                         lines = text.strip('\n').split('\n')
                         pages.extend(self._paginate_slide(lines, line_end))
                         break
+                    count += 1
             else:
                 # Clean up line endings.
                 pages = self._paginate_slide(text.split('\n'), line_end)

=== modified file 'openlp/plugins/songusage/forms/songusagedetailform.py'
--- openlp/plugins/songusage/forms/songusagedetailform.py	2014-03-21 18:23:35 +0000
+++ openlp/plugins/songusage/forms/songusagedetailform.py	2014-11-15 09:52:34 +0000
@@ -99,7 +99,7 @@
         report_file_name = os.path.join(path, file_name)
         file_handle = None
         try:
-            file_handle = open(report_file_name, 'w')
+            file_handle = open(report_file_name, 'wb')
             for instance in usage:
                 record = '\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",' \
                     '\"%s\",\"%s\"\n' % \

=== modified file 'tests/functional/openlp_core_lib/test_renderer.py'
--- tests/functional/openlp_core_lib/test_renderer.py	2014-07-02 18:58:52 +0000
+++ tests/functional/openlp_core_lib/test_renderer.py	2014-11-15 09:52:34 +0000
@@ -34,7 +34,7 @@
 from PyQt4 import QtCore
 
 from openlp.core.common import Registry
-from openlp.core.lib import Renderer, ScreenList
+from openlp.core.lib import Renderer, ScreenList, ServiceItem
 
 from tests.interfaces import MagicMock
 
@@ -109,3 +109,55 @@
 
         # THEN: The word lists should be the same.
         self.assertListEqual(result_words, expected_words)
+
+    def format_slide_logical_split_test(self):
+        """
+        Test that a line with text and a logic break does not break the renderer just returns the input
+        """
+        # GIVEN: A line of text
+        renderer = Renderer()
+        renderer.empty_height = 25
+        given_line = 'a\n[---]\nb'
+        expected_words = ['a<br>[---]<br>b']
+        service_item = ServiceItem(None)
+
+        # WHEN: Split the line
+
+        result_words = renderer.format_slide(given_line, service_item)
+
+        # THEN: The word lists should be the same.
+        self.assertListEqual(result_words, expected_words)
+
+    def format_slide_blank_before_split_test(self):
+        """
+        Test that a line with blanks before the logical split at handled
+        """
+        # GIVEN: A line of text
+        renderer = Renderer()
+        renderer.empty_height = 25
+        given_line = '\n       [---]\n'
+        expected_words = ['<br>       [---]']
+        service_item = ServiceItem(None)
+
+        # WHEN: Split the line
+        result_words = renderer.format_slide(given_line, service_item)
+
+        # THEN: The blanks have been removed.
+        self.assertListEqual(result_words, expected_words)
+
+    def format_slide_blank_after_split_test(self):
+        """
+        Test that a line with blanks before the logical split at handled
+        """
+        # GIVEN: A line of text
+        renderer = Renderer()
+        renderer.empty_height = 25
+        given_line = '\n[---]  \n'
+        expected_words = ['<br>[---]  ']
+        service_item = ServiceItem(None)
+
+        # WHEN: Split the line
+        result_words = renderer.format_slide(given_line, service_item)
+
+        # THEN: The blanks have been removed.
+        self.assertListEqual(result_words, expected_words)