[blivet 2/2] Accept both English and localized sizes in Size specs.
David Shea
dshea at redhat.com
Tue Jan 21 14:36:27 UTC 2014
On 01/21/2014 02:31 AM, Vratislav Podzimek wrote:
> On Mon, 2014-01-20 at 17:26 -0500, David Shea wrote:
>> _parseSpec will accept both English and localized size specs, either
>> either a period or a localized radix character. Size.humanReadable will
> Double 'either'.
Oops, fixed.
>
>> always return the localized version of a Size.
>> ---
>> blivet/size.py | 86 +++++++++++++++++++++++++++++++++++++++---------------
>> tests/size_test.py | 42 ++++++++++++++++++++++++++
>> 2 files changed, 105 insertions(+), 23 deletions(-)
>>
>> diff --git a/blivet/size.py b/blivet/size.py
>> index a0d572d..3969c15 100644
>> --- a/blivet/size.py
>> +++ b/blivet/size.py
>> @@ -20,6 +20,8 @@
>> # Red Hat Author(s): David Cantrell <dcantrell at redhat.com>
>>
>> import re
>> +import string
>> +import locale
>>
>> from decimal import Decimal
>> from decimal import InvalidOperation
>> @@ -54,17 +56,30 @@ _binaryPrefix = [(1024, N_("kibi"), N_("Ki")),
>> _bytes = [N_('B'), N_('b'), N_('byte'), N_('bytes')]
>> _prefixes = _binaryPrefix + _decimalPrefix
>>
>> -def _makeSpecs(prefix, abbr):
>> +_ASCIIlower_table = string.maketrans(string.ascii_uppercase, string.ascii_lowercase)
>> +def _lowerASCII(s):
>> + """Convert a string to lowercase using only ASCII character definitions."""
>> + return string.translate(s, _ASCIIlower_table)
>> +
>> +def _makeSpecs(prefix, abbr, xlate):
>> """ Internal method used to generate a list of specifiers. """
>> specs = []
>>
>> if prefix:
>> - specs.append(prefix.lower() + _("byte"))
>> - specs.append(prefix.lower() + _("bytes"))
>> + if xlate:
>> + specs.append(prefix.lower() + _("byte").decode("utf-8"))
>> + specs.append(prefix.lower() + _("bytes").decode("utf-8"))
>> + else:
>> + specs.append(_lowerASCII(prefix) + "byte")
>> + specs.append(_lowerASCII(prefix) + "bytes")
>>
>> if abbr:
>> - specs.append(abbr.lower() + _("b"))
>> - specs.append(abbr.lower())
>> + if xlate:
>> + specs.append(abbr.lower() + _("b").decode("utf-8"))
>> + specs.append(abbr.lower())
>> + else:
>> + specs.append(_lowerASCII(abbr) + "b")
>> + specs.append(_lowerASCII(abbr))
>>
>> return specs
>>
>> @@ -73,11 +88,15 @@ def _parseSpec(spec):
>> if not spec:
>> raise ValueError("invalid size specification", spec)
>>
>> - # This regex isn't ideal, since \w matches both letters and digits,
>> - # but python doesn't provide a means to match only Unicode letters.
>> - # Probably the worst that will come of it is that bad specs will fail
>> - # more confusingly.
>> - m = re.match(r'(-?\s*[0-9.]+)\s*(\w*)$', spec.decode("utf-8").strip(), flags=re.UNICODE)
>> + # Replace the localized radix character with a .
>> + radix = locale.nl_langinfo(locale.RADIXCHAR)
>> + if radix != '.':
>> + spec = spec.replace(radix, '.')
>> +
>> + # Match the string using only digit/space/not-space, since the
>> + # string might be non-English and contain non-letter characters
>> + # that Python doesn't understand as parts of words.
>> + m = re.match(r'(-?\s*[0-9.]+)\s*([^\s]*)$', spec.strip())
>> if not m:
>> raise ValueError("invalid size specification", spec)
>>
>> @@ -86,14 +105,30 @@ def _parseSpec(spec):
>> except InvalidOperation:
>> raise ValueError("invalid size specification", spec)
>>
>> - specifier = m.groups()[1].lower()
>> - bytes_xlated = [_(b) for b in _bytes]
>> - if not specifier or specifier in bytes_xlated:
>> + # Only attempt to parse as English if all characters are ASCII
>> + try:
>> + specifier = _lowerASCII(str(m.groups()[1].decode('ascii').lower()))
> Could you please try to split this into multiple lines? If an exception
> was raised in such line, it would be really hard to tell in which part.
> Also assigning results of independent steps into properly named
> variables will make the code clearer.
Ok.
>
>> + except UnicodeDecodeError:
>> + pass
>> + else:
>> + if specifier in _bytes or not specifier:
>> + return size
>> +
>> + for factor, prefix, abbr in _prefixes:
>> + check = _makeSpecs(prefix, abbr, False)
>> +
>> + if specifier in check:
>> + return size * factor
>> +
>> + # No English match found, try localized size specs
>> + xlated_bytes = [_(b).decode("utf-8") for b in _bytes]
>> + specifier = m.groups()[1].decode("utf-8").lower()
>> + if specifier in xlated_bytes:
>> return size
>>
>> - prefixes_xlated = [_(p) for p in _prefixes]
>> - for factor, prefix, abbr in prefixes_xlated:
>> - check = _makeSpecs(prefix, abbr)
>> + xlated_prefixes = [(p[0], _(p[1]).decode("utf-8"), _(p[2]).decode("utf-8")) for p in _prefixes]
> A thing for another patch probably, but could _prefixes be a list of
> namedtuples with named fields? This lines looks like magic to me. Also
> having a function xlate_prefix and mapping it on _prefixes may be
> better, hiding the details of the implementation from the code that
> doesn't have to care about it.
Yeah, it'd be better off in a function at the least.
>
>> + for factor, prefix, abbr in xlated_prefixes:
>> + check = _makeSpecs(prefix, abbr, True)
>>
>> if specifier in check:
>> return size * factor
>> @@ -192,13 +227,11 @@ class Size(Decimal):
>> """
>> spec = spec.lower()
>>
>> - bytes_xlated = [_(b) for b in _bytes]
>> - if spec in bytes_xlated:
>> + if spec in _bytes:
>> return self
>>
>> - prefixes_xlated = [_(p) for p in _prefixes]
>> - for factor, prefix, abbr in prefixes_xlated:
>> - check = _makeSpecs(prefix, abbr)
>> + for factor, prefix, abbr in _prefixes:
>> + check = _makeSpecs(prefix, abbr, False)
>>
>> if spec in check:
>> return Decimal(self / Decimal(factor))
>> @@ -221,7 +254,7 @@ class Size(Decimal):
>> if abs(Decimal(check)) < 1000:
>> return "%s %s" % (check, _("B"))
>>
>> - prefixes_xlated = [_(p) for p in _prefixes]
>> + prefixes_xlated = [(p[0], _(p[1]), _(p[2])) for p in _prefixes]
>> for factor, prefix, abbr in prefixes_xlated:
>> newcheck = super(Size, self).__div__(Decimal(factor))
>>
>> @@ -229,6 +262,8 @@ class Size(Decimal):
>> # nice value, use this factor, prefix and abbr
>> break
>>
>> + # Format the value with '.' as the decimal separator
>> + # If necessary, substitute with a localized separator before returning
>> if places is not None:
>> newcheck_str = str(newcheck)
>> retval = newcheck_str
>> @@ -238,13 +273,18 @@ class Size(Decimal):
>> else:
>> retval = self._trimEnd(str(newcheck))
>>
>> + radix = locale.nl_langinfo(locale.RADIXCHAR)
>> +
>> if max_places is not None:
>> (whole, point, fraction) = retval.partition(".")
>> if point and len(fraction) > max_places:
>> if max_places == 0:
>> retval = whole
>> else:
>> - retval = "%s.%s" % (whole, fraction[:max_places])
>> + retval = "%s%s%s" % (whole, radix, fraction[:max_places])
>> +
>> + if radix != '.':
>> + retval = retval.replace('.', radix)
>>
>> if abbr:
>> return retval + " " + abbr + _("B")
>> diff --git a/tests/size_test.py b/tests/size_test.py
>> index 30bc6eb..fc1df2b 100644
>> --- a/tests/size_test.py
>> +++ b/tests/size_test.py
>> @@ -87,6 +87,48 @@ class SizeTestCase(unittest.TestCase):
>> self.assertEquals(Size(spec="%s KiB" % (1/1025.0,)), Size(bytes=0))
>> self.assertEquals(Size(spec="%s KiB" % (1/1023.0,)), Size(bytes=1))
>>
>> + def testTranslated(self):
>> + import locale
>> + import os
>> + from blivet.i18n import _
>> + import gettext
>> +
>> + saved_lang = os.environ.get('LANG', None)
>> +
>> + # es_ES uses latin-characters but a comma as the radix separator
>> + # kk_KZ uses non-latin characters and is case-sensitive
>> + # te_IN uses a lot of non-letter modifier characters
>> + test_langs = ["es_ES.UTF-8", "kk_KZ.UTF-8", "ml_IN.UTF-8"]
>> +
>> + s = Size(spec="56.19 MiB")
>> + for lang in test_langs:
>> + os.environ['LANG'] = lang
>> + locale.setlocale(locale.LC_ALL, '')
>> +
>> + # Check English parsing
>> + self.assertEquals(s, Size(spec="56.19 MiB"))
>> +
>> + # Check native parsing
>> + self.assertEquals(s, Size(spec="56.19 %s%s" % (_("Mi"), _("B"))))
>> +
>> + # Check native parsing, all lowercase
>> + self.assertEquals(s, Size(spec=("56.19 %s%s" % (_("Mi"), _("B"))).lower()))
>> +
>> + # Check native parsing, all uppercase
>> + self.assertEquals(s, Size(spec=("56.19 %s%s" % (_("Mi"), _("B"))).upper()))
>> +
>> + # If the radix separator is not a period, repeat the tests with the
>> + # native separator
>> + radix = locale.nl_langinfo(locale.RADIXCHAR)
>> + if radix != '.':
>> + self.assertEquals(s, Size(spec="56%s19 MiB" % radix))
>> + self.assertEquals(s, Size(spec="56%s19 %s%s" % (radix, _("Mi"), _("B"))))
>> + self.assertEquals(s, Size(spec=("56%s19 %s%s" % (radix, _("Mi"), _("B"))).lower()))
>> + self.assertEquals(s, Size(spec=("56%s19 %s%s" % (radix, _("Mi"), _("B"))).upper()))
>> +
>> + os.environ['LANG'] = saved_lang
>> + locale.setlocale(locale.LC_ALL, '')
> You should probably restore the $LC_ALL value here as well not just set it to an empty string.
>
Calling setlocale with an empty string sets the locale based on the
environment, which is being restored.
More information about the anaconda-patches
mailing list