Issue #1222: locale.format() bug when the thousands separator is a space character.

3 files changed, 131 insertions, 33 deletions

diff --git a/Lib/locale.py b/Lib/locale.py
index fd948a7..f853867 100644
--- a/Lib/locale.py
+++ b/Lib/locale.py
@@ -108,6 +108,19 @@ def localeconv():
 # Author: Martin von Loewis
 # improved by Georg Brandl
 
+# Iterate over grouping intervals
+def _grouping_intervals(grouping):
+    for interval in grouping:
+        # if grouping is -1, we are done
+        if interval == CHAR_MAX:
+            return
+        # 0: re-use last group ad infinitum
+        if interval == 0:
+            while True:
+                yield last_interval
+        yield interval
+        last_interval = interval
+
 #perform the grouping from right to left
 def _group(s, monetary=False):
     conv = localeconv()
@@ -117,35 +130,41 @@ def _group(s, monetary=False):
         return (s, 0)
     result = ""
     seps = 0
-    spaces = ""
     if s[-1] == ' ':
-        sp = s.find(' ')
-        spaces = s[sp:]
-        s = s[:sp]
-    while s and grouping:
-        # if grouping is -1, we are done
-        if grouping[0] == CHAR_MAX:
+        stripped = s.rstrip()
+        right_spaces = s[len(stripped):]
+        s = stripped
+    else:
+        right_spaces = ''
+    left_spaces = ''
+    groups = []
+    for interval in _grouping_intervals(grouping):
+        if not s or s[-1] not in "0123456789":
+            # only non-digit characters remain (sign, spaces)
+            left_spaces = s
+            s = ''
             break
-        # 0: re-use last group ad infinitum
-        elif grouping[0] != 0:
-            #process last group
-            group = grouping[0]
-            grouping = grouping[1:]
-        if result:
-            result = s[-group:] + thousands_sep + result
-            seps += 1
-        else:
-            result = s[-group:]
-        s = s[:-group]
-        if s and s[-1] not in "0123456789":
-            # the leading string is only spaces and signs
-            return s + result + spaces, seps
-    if not result:
-        return s + spaces, seps
+        groups.append(s[-interval:])
+        s = s[:-interval]
     if s:
-        result = s + thousands_sep + result
-        seps += 1
-    return result + spaces, seps
+        groups.append(s)
+    groups.reverse()
+    return (
+        left_spaces + thousands_sep.join(groups) + right_spaces,
+        len(groups) - 1
+    )
+
+# Strip a given amount of excess padding from the given string
+def _strip_padding(s, amount):
+    lpos = 0
+    while amount and s[lpos] == ' ':
+        lpos += 1
+        amount -= 1
+    rpos = len(s) - 1
+    while amount and s[rpos] == ' ':
+        rpos -= 1
+        amount -= 1
+    return s[lpos:rpos+1]
 
 def format(percent, value, grouping=False, monetary=False, *additional):
     """Returns the locale-aware substitution of a %? specifier
@@ -170,14 +189,14 @@ def format(percent, value, grouping=False, monetary=False, *additional):
         decimal_point = localeconv()[monetary and 'mon_decimal_point'
                                               or 'decimal_point']
         formatted = decimal_point.join(parts)
-        while seps:
-            sp = formatted.find(' ')
-            if sp == -1: break
-            formatted = formatted[:sp] + formatted[sp+1:]
-            seps -= 1
+        if seps:
+            formatted = _strip_padding(formatted, seps)
     elif percent[-1] in 'diu':
+        seps = 0
         if grouping:
-            formatted = _group(formatted, monetary=monetary)[0]
+            formatted, seps = _group(formatted, monetary=monetary)
+        if seps:
+            formatted = _strip_padding(formatted, seps)
     return formatted
 
 import re, operator
diff --git a/Lib/test/test_locale.py b/Lib/test/test_locale.py
index 0ff3194..2612c79 100644
--- a/Lib/test/test_locale.py
+++ b/Lib/test/test_locale.py
@@ -105,6 +105,32 @@ class EnUSCookedTest(BaseCookedTest):
     }
 
 
+class FrFRCookedTest(BaseCookedTest):
+    # A cooked "fr_FR" locale with a space character as decimal separator
+    # and a non-ASCII currency symbol.
+
+    cooked_values = {
+        'currency_symbol': '\xe2\x82\xac',
+        'decimal_point': ',',
+        'frac_digits': 2,
+        'grouping': [3, 3, 0],
+        'int_curr_symbol': 'EUR ',
+        'int_frac_digits': 2,
+        'mon_decimal_point': ',',
+        'mon_grouping': [3, 3, 0],
+        'mon_thousands_sep': ' ',
+        'n_cs_precedes': 0,
+        'n_sep_by_space': 1,
+        'n_sign_posn': 1,
+        'negative_sign': '-',
+        'p_cs_precedes': 0,
+        'p_sep_by_space': 1,
+        'p_sign_posn': 1,
+        'positive_sign': '',
+        'thousands_sep': ' '
+    }
+
+
 class BaseFormattingTest(object):
     #
     # Utility functions for formatting tests
@@ -152,6 +178,12 @@ class EnUSNumberFormatting(BaseFormattingTest):
         self._test_format("%+d", 4200, grouping=True, out='+4%s200' % self.sep)
         self._test_format("%+d", -4200, grouping=True, out='-4%s200' % self.sep)
 
+    def test_integer_grouping_and_padding(self):
+        self._test_format("%10d", 4200, grouping=True,
+            out=('4%s200' % self.sep).rjust(10))
+        self._test_format("%-10d", -4200, grouping=True,
+            out=('-4%s200' % self.sep).ljust(10))
+
     def test_simple(self):
         self._test_format("%f", 1024, grouping=0, out='1024.000000')
         self._test_format("%f", 102, grouping=0, out='102.000000')
@@ -223,6 +255,49 @@ class TestCNumberFormatting(CCookedTest, BaseFormattingTest):
         self._test_format("%9.2f", 12345.67, grouping=True, out=' 12345.67')
 
 
+class TestFrFRNumberFormatting(FrFRCookedTest, BaseFormattingTest):
+    # Test number formatting with a cooked "fr_FR" locale.
+
+    def test_decimal_point(self):
+        self._test_format("%.2f", 12345.67, out='12345,67')
+
+    def test_grouping(self):
+        self._test_format("%.2f", 345.67, grouping=True, out='345,67')
+        self._test_format("%.2f", 12345.67, grouping=True, out='12 345,67')
+
+    def test_grouping_and_padding(self):
+        self._test_format("%6.2f", 345.67, grouping=True, out='345,67')
+        self._test_format("%7.2f", 345.67, grouping=True, out=' 345,67')
+        self._test_format("%8.2f", 12345.67, grouping=True, out='12 345,67')
+        self._test_format("%9.2f", 12345.67, grouping=True, out='12 345,67')
+        self._test_format("%10.2f", 12345.67, grouping=True, out=' 12 345,67')
+        self._test_format("%-6.2f", 345.67, grouping=True, out='345,67')
+        self._test_format("%-7.2f", 345.67, grouping=True, out='345,67 ')
+        self._test_format("%-8.2f", 12345.67, grouping=True, out='12 345,67')
+        self._test_format("%-9.2f", 12345.67, grouping=True, out='12 345,67')
+        self._test_format("%-10.2f", 12345.67, grouping=True, out='12 345,67 ')
+
+    def test_integer_grouping(self):
+        self._test_format("%d", 200, grouping=True, out='200')
+        self._test_format("%d", 4200, grouping=True, out='4 200')
+
+    def test_integer_grouping_and_padding(self):
+        self._test_format("%4d", 4200, grouping=True, out='4 200')
+        self._test_format("%5d", 4200, grouping=True, out='4 200')
+        self._test_format("%10d", 4200, grouping=True, out='4 200'.rjust(10))
+        self._test_format("%-4d", 4200, grouping=True, out='4 200')
+        self._test_format("%-5d", 4200, grouping=True, out='4 200')
+        self._test_format("%-10d", 4200, grouping=True, out='4 200'.ljust(10))
+
+    def test_currency(self):
+        euro = u'\u20ac'.encode('utf-8')
+        self._test_currency(50000, "50000,00 " + euro)
+        self._test_currency(50000, "50 000,00 " + euro, grouping=True)
+        # XXX is the trailing space a bug?
+        self._test_currency(50000, "50 000,00 EUR ",
+            grouping=True, international=True)
+
+
 class TestStringMethods(BaseLocalizedTest):
     locale_type = locale.LC_CTYPE
 
@@ -277,7 +352,8 @@ def test_main():
     tests = [
         TestMiscellaneous,
         TestEnUSNumberFormatting,
-        TestCNumberFormatting
+        TestCNumberFormatting,
+        TestFrFRNumberFormatting,
     ]
     # TestSkipped can't be raised inside unittests, handle it manually instead
     try:
diff --git a/Misc/NEWS b/Misc/NEWS
index c9f77b9..d5ddd10 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -171,6 +171,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #1222: locale.format() bug when the thousands separator is a space
+  character.
+
 - Issue #5472: Fixed distutils.test_util tear down. Original patch by
   Tim Golden.