openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Review: Needs Fixing

I think we should leave most of these changes in trunk-2.2, and not merge into trunk.
IMHO we should wait for PyInstaller to come up with a fix, and we should drop support for python 3.3.
Also see inline comments...

Diff comments:

> === modified file 'openlp/core/ui/media/vendor/vlc.py'
> --- openlp/core/ui/media/vendor/vlc.py	2015-09-03 19:11:08 +0000
> +++ openlp/core/ui/media/vendor/vlc.py	2015-11-21 08:42:11 +0000
> @@ -107,6 +107,7 @@
>          except OSError:  # may fail
>              dll = ctypes.CDLL('libvlc.so.5')
>      elif sys.platform.startswith('win'):
> +        ctypes.windll.kernel32.SetDllDirectoryW(None)

This one should be fixed by PyInstaller

>          p = find_library('libvlc.dll')
>          if p is None:
>              try:  # some registry settings
> 
> === modified file 'openlp/plugins/songs/lib/songselect.py'
> --- openlp/plugins/songs/lib/songselect.py	2015-07-04 21:09:59 +0000
> +++ openlp/plugins/songs/lib/songselect.py	2015-11-21 08:42:11 +0000
> @@ -23,10 +23,13 @@
>  The :mod:`~openlp.plugins.songs.lib.songselect` module contains the SongSelect importer itself.
>  """
>  import logging
> +import sys
>  from http.cookiejar import CookieJar
>  from urllib.parse import urlencode
>  from urllib.request import HTTPCookieProcessor, URLError, build_opener
>  from html.parser import HTMLParser
> +if sys.version_info > (3, 4):
> +    from html import unescape

Remove the "if", keep the import

>  
>  
>  from bs4 import BeautifulSoup, NavigableString
> @@ -129,11 +132,18 @@
>              if not search_results:
>                  break
>              for result in search_results:
> -                song = {
> -                    'title': self.html_parser.unescape(result.find('h3').string),
> -                    'authors': [self.html_parser.unescape(author.string) for author in result.find_all('li')],
> -                    'link': BASE_URL + result.find('a')['href']
> -                }
> +                if sys.version_info > (3, 4):
> +                    song = {
> +                        'title': unescape(result.find('h3').string),
> +                        'authors': [unescape(author.string) for author in result.find_all('li')],
> +                        'link': BASE_URL + result.find('a')['href']
> +                    }
> +                else:
> +                    song = {
> +                        'title': self.html_parser.unescape(result.find('h3').string),
> +                        'authors': [self.html_parser.unescape(author.string) for author in result.find_all('li')],
> +                        'link': BASE_URL + result.find('a')['href']
> +                    }

Keep the content of the "if", not the "else"

>                  if callback:
>                      callback(song)
>                  songs.append(song)
> @@ -167,7 +177,10 @@
>          if callback:
>              callback()
>          song['copyright'] = '/'.join([li.string for li in song_page.find('ul', 'copyright').find_all('li')])
> -        song['copyright'] = self.html_parser.unescape(song['copyright'])
> +        if sys.version_info > (3, 4):
> +            song['copyright'] = unescape(song['copyright'])
> +        else:
> +            song['copyright'] = self.html_parser.unescape(song['copyright'])

Keep the content of the "if", not the "else"

>          song['ccli_number'] = song_page.find('ul', 'info').find('li').string.split(':')[1].strip()
>          song['verses'] = []
>          verses = lyrics_page.find('section', 'lyrics').find_all('p')
> @@ -180,9 +193,15 @@
>                  else:
>                      verse['lyrics'] += '\n'
>              verse['lyrics'] = verse['lyrics'].strip(' \n\r\t')
> -            song['verses'].append(self.html_parser.unescape(verse))
> +            if sys.version_info > (3, 4):
> +                song['verses'].append(unescape(verse))
> +            else:
> +                song['verses'].append(self.html_parser.unescape(verse))

Keep the content of the "if", not the "else"

>          for counter, author in enumerate(song['authors']):
> -            song['authors'][counter] = self.html_parser.unescape(author)
> +            if sys.version_info > (3, 4):
> +                song['authors'][counter] = unescape(author)
> +            else:
> +                song['authors'][counter] = self.html_parser.unescape(author)

Keep the content of the "if", not the "else"

>          return song
>  
>      def save_song(self, song):


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