openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing

A little breathing room that needs to be sorted out, but otherwise looks fine.

Diff comments:

> === 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-16 20:48:42 +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/core/ui/firsttimeform.py'
> --- openlp/core/ui/firsttimeform.py	2014-11-01 10:38:33 +0000
> +++ openlp/core/ui/firsttimeform.py	2014-11-16 20:48:42 +0000
> @@ -95,6 +95,17 @@
>          """
>          self.application.process_events()
>          if self.currentId() == FirstTimePage.Plugins:
> +            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())
> +                self.presentation_check_box.setChecked(self.plugin_manager.get_plugin_by_name(
> +                    'presentations').is_active())
> +                self.image_check_box.setChecked(self.plugin_manager.get_plugin_by_name('images').is_active())
> +                self.media_check_box.setChecked(self.plugin_manager.get_plugin_by_name('media').is_active())
> +                self.remote_check_box.setChecked(self.plugin_manager.get_plugin_by_name('remotes').is_active())
> +                self.custom_check_box.setChecked(self.plugin_manager.get_plugin_by_name('custom').is_active())
> +                self.song_usage_check_box.setChecked(self.plugin_manager.get_plugin_by_name('songusage').is_active())
> +                self.alert_check_box.setChecked(self.plugin_manager.get_plugin_by_name('alerts').is_active())
>              if not self.web_access:
>                  return FirstTimePage.NoInternet
>              else:
> 
> === 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-16 20:48:42 +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-16 20:48:42 +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
> +

Breathing room?

> +        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)
> 


-- 
https://code.launchpad.net/~trb143/openlp/bug-1202677/+merge/241904
Your team OpenLP Core is subscribed to branch lp:openlp.