openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing

Nicely done! Just some comments to address, and then I think we're good to do.

Diff comments:

> === modified file 'openlp/plugins/songs/lib/importer.py'
> --- openlp/plugins/songs/lib/importer.py	2016-03-20 08:28:41 +0000
> +++ openlp/plugins/songs/lib/importer.py	2016-03-30 17:43:25 +0000
> @@ -313,9 +313,9 @@
>          },
>          ProPresenter: {
>              'class': ProPresenterImport,
> -            'name': 'ProPresenter 4',
> +            'name': 'ProPresenter 4 -> 6',

I would rather say "ProPresenter 4, 5 and 6"

>              'prefix': 'proPresenter',
> -            'filter': '%s (*.pro4)' % translate('SongsPlugin.ImportWizardForm', 'ProPresenter 4 Song Files')
> +            'filter': '%s (*.pro4 *.pro5 *.pro6)' % translate('SongsPlugin.ImportWizardForm', 'ProPresenter Song Files')
>          },
>          SongBeamer: {
>              'class': SongBeamerImport,
> 
> === modified file 'openlp/plugins/songs/lib/importers/propresenter.py'
> --- openlp/plugins/songs/lib/importers/propresenter.py	2015-12-31 22:46:06 +0000
> +++ openlp/plugins/songs/lib/importers/propresenter.py	2016-03-30 17:43:25 +0000
> @@ -52,23 +52,82 @@
>  
>      def process_song(self, root, filename):
>          self.set_defaults()
> -        self.title = os.path.basename(filename).rstrip('.pro4')
> -        self.copyright = root.get('CCLICopyrightInfo')
> +        self.title = os.path.basename(filename)
> +
> +        # Extract ProPresenter versionNumber
> +        try:
> +            self.version = int(root.get('versionNumber'))
> +        except ValueError:
> +            log.debug('ProPresenter versionNumber invalid or missing')
> +            return
> +
> +        # Common settings
>          self.comments = root.get('notes')
> -        self.ccli_number = root.get('CCLILicenseNumber')
> -        for author_key in ['author', 'artist', 'CCLIArtistCredits']:
> +        for author_key in ['author', 'CCLIAuthor', 'artist', 'CCLIArtistCredits']:
>              author = root.get(author_key)
> -            if len(author) > 0:
> +            if author and len(author) > 0:
>                  self.parse_author(author)
> -        count = 0
> -        for slide in root.slides.RVDisplaySlide:
> -            count += 1
> -            if not hasattr(slide.displayElements, 'RVTextElement'):
> -                log.debug('No text found, may be an image slide')
> -                continue
> -            RTFData = slide.displayElements.RVTextElement.get('RTFData')
> -            rtf = base64.standard_b64decode(RTFData)
> -            words, encoding = strip_rtf(rtf.decode())
> -            self.add_verse(words, "v%d" % count)
> +
> +        # ProPresenter 4
> +        if(self.version >= 400 and self.version < 500):
> +            if self.title.endswith('.pro4'):
> +                self.title = self.title[:-5]
> +            self.copyright = root.get('CCLICopyrightInfo')
> +            self.ccli_number = root.get('CCLILicenseNumber')

Can't you refactor these three lines in each of the 4, 5 and 6 sections, since they seem to be identical except for the file extension (which you can probably just ignore anyway, since you're not dealing with it)?

> +            count = 0
> +            for slide in root.slides.RVDisplaySlide:
> +                count += 1
> +                if not hasattr(slide.displayElements, 'RVTextElement'):
> +                    log.debug('No text found, may be an image slide')
> +                    continue
> +                RTFData = slide.displayElements.RVTextElement.get('RTFData')
> +                rtf = base64.standard_b64decode(RTFData)
> +                words, encoding = strip_rtf(rtf.decode())
> +                self.add_verse(words, "v%d" % count)
> +
> +        # ProPresenter 5
> +        elif(self.version >= 500 and self.version < 600):
> +            if self.title.endswith('.pro5'):
> +                self.title = self.title[:-5]
> +            self.copyright = root.get('CCLICopyrightInfo')
> +            self.ccli_number = root.get('CCLILicenseNumber')
> +            count = 0
> +            for group in root.groups.RVSlideGrouping:
> +                for slide in group.slides.RVDisplaySlide:
> +                    count += 1
> +                    if not hasattr(slide.displayElements, 'RVTextElement'):
> +                        log.debug('No text found, may be an image slide')
> +                        continue
> +                    RTFData = slide.displayElements.RVTextElement.get('RTFData')
> +                    rtf = base64.standard_b64decode(RTFData)
> +                    words, encoding = strip_rtf(rtf.decode())
> +                    self.add_verse(words, "v%d" % count)
> +
> +        # ProPresenter 6
> +        elif(self.version >= 600 and self.version < 700):
> +            if self.title.endswith('.pro6'):
> +                self.title = self.title[:-5]
> +            self.copyright = root.get('CCLICopyrightYear')
> +            self.ccli_number = root.get('CCLISongNumber')
> +            count = 0
> +            for group in root.array.RVSlideGrouping:
> +                for slide in group.array.RVDisplaySlide:
> +                    count += 1
> +                    for item in slide.array:
> +                        if not (item.get('rvXMLIvarName') == "displayElements"):
> +                            continue
> +                        if not hasattr(item, 'RVTextElement'):
> +                            log.debug('No text found, may be an image slide')
> +                            continue
> +                        for contents in item.RVTextElement.NSString:
> +                            b64Data = contents.text
> +                            data = base64.standard_b64decode(b64Data)
> +                            words = None
> +                            if(contents.get('rvXMLIvarName') == "RTFData"):
> +                                words, encoding = strip_rtf(data.decode())
> +                                break
> +                        if words:
> +                            self.add_verse(words, "v%d" % count)
> +
>          if not self.finish():
>              self.log_error(self.import_source)


-- 
https://code.launchpad.net/~knightrider0xd/openlp/propresenter_5-6_import/+merge/290508
Your team OpenLP Core is subscribed to branch lp:openlp.