openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

See below

Diff comments:

> 
> === modified file 'openlp/core/lib/__init__.py'
> --- openlp/core/lib/__init__.py	2016-08-12 19:16:12 +0000
> +++ openlp/core/lib/__init__.py	2016-10-17 04:44:46 +0000
> @@ -310,30 +310,23 @@
>  
>  def create_separated_list(string_list):
>      """
> -    Returns a string that represents a join of a list of strings with a localized separator. This function corresponds
> -
> -    to QLocale::createSeparatedList which was introduced in Qt 4.8 and implements the algorithm from
> -    http://www.unicode.org/reports/tr35/#ListPatterns
> -
> -     :param string_list: List of unicode strings
> +    Returns a string that represents a join of a list of strings with a localized separator.
> +    Localized separation will be done via the translate() function by the translators.
> +
> +    :param string_list: List of unicode strings
> +    :return: Formatted string
>      """
> -    if LooseVersion(Qt.PYQT_VERSION_STR) >= LooseVersion('4.9') and LooseVersion(Qt.qVersion()) >= LooseVersion('4.8'):
> -        return QtCore.QLocale().createSeparatedList(string_list)
> -    if not string_list:
> -        return ''
> -    elif len(string_list) == 1:
> -        return string_list[0]
> -    # TODO: Verify mocking of translate() test before conversion
> -    elif len(string_list) == 2:
> -        return translate('OpenLP.core.lib', '%s and %s',
> -                         'Locale list separator: 2 items') % (string_list[0], string_list[1])
> +    list_length = len(string_list)
> +    if list_length == 1:
> +        return_list = string_list[0]

return_list implies the field is a list when it is a string.

> +    elif list_length == 2:
> +        return_list = translate('OpenLP.core.lib', '{one} and {two}').format(one=string_list[0], two=string_list[1])

should we not just translate "and" once and then build the strings as there is a high chance of the {} getting lost.

> +    elif list_length > 2:
> +        return_list = translate('OpenLP.core.lib', '{first}, and {last}').format(first=', '.join(string_list[:-1]),
> +                                                                                 last=string_list[-1])
>      else:
> -        merged = translate('OpenLP.core.lib', '%s, and %s',
> -                           'Locale list separator: end') % (string_list[-2], string_list[-1])
> -        for index in reversed(list(range(1, len(string_list) - 2))):
> -            merged = translate('OpenLP.core.lib', '%s, %s',
> -                               'Locale list separator: middle') % (string_list[index], merged)
> -        return translate('OpenLP.core.lib', '%s, %s', 'Locale list separator: start') % (string_list[0], merged)
> +        return_list = ""

Should be ''

> +    return return_list
>  
>  
>  from .exceptions import ValidationError


-- 
https://code.launchpad.net/~suutari-olli/openlp/bug-fixes-2-4-3/+merge/308596
Your team OpenLP Core is subscribed to branch lp:openlp.