[Help-smalltalk] [PATCH] i18n: Fix printing of integers and floats

[Holger Hans Peter Freyther]

The printing of Integers didn't work as the stream was
attempted to be indexed out of bounds and the calculation
of the number of required groups was wrong in many cases.

2015-10-18  Holger Hans Peter Freyther  <address@hidden>

        * package.xml: Add LcNumericTest.st and testcase.
        * Number.st: Fix calculation in
        >>#computeNumberOfGroups. Support integer
        printing in >>#basicPrint:on:.
        * LcNumericTest.st: Add new testcase.
---
 ChangeLog                      |   4 ++
 packages/i18n/ChangeLog        |   8 +++
 packages/i18n/LcNumericTest.st | 117 +++++++++++++++++++++++++++++++++++++++++
 packages/i18n/Makefile.frag    |   2 +-
 packages/i18n/Numbers.st       |  33 ++++++++++--
 packages/i18n/package.xml      |   5 ++
 tests/testsuite.at             |   1 +
 7 files changed, 164 insertions(+), 6 deletions(-)
 create mode 100644 packages/i18n/LcNumericTest.st

diff --git a/ChangeLog b/ChangeLog
index 2875294..63cfd08 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2015-10-18  Holger Hans Peter Freyther  <address@hidden>
+
+       * tests/testsuite.at: Add I18N test to the testsuite.
+
 2015-05-21  Holger Hans Peter Freyther  <address@hidden>
 
        * configure.ac: Move from AC_CHECK_FUNCS_ONCE to AC_CHECK_DECLS
diff --git a/packages/i18n/ChangeLog b/packages/i18n/ChangeLog
index c66bc1f..9701dca 100644
--- a/packages/i18n/ChangeLog
+++ b/packages/i18n/ChangeLog
@@ -1,3 +1,11 @@
+2015-10-18  Holger Hans Peter Freyther  <address@hidden>
+
+       * package.xml: Add LcNumericTest.st and testcase.
+       * Number.st: Fix calculation in
+       >>#computeNumberOfGroups. Support integer
+       printing in >>#basicPrint:on:.
+       * LcNumericTest.st: Add new testcase.
+
 2010-12-04  Paolo Bonzini  <address@hidden>
 
        * package.xml: Remove now superfluous <file> tags.
diff --git a/packages/i18n/LcNumericTest.st b/packages/i18n/LcNumericTest.st
new file mode 100644
index 0000000..496c3de
--- /dev/null
+++ b/packages/i18n/LcNumericTest.st
@@ -0,0 +1,117 @@
+"======================================================================
+|
+|   Localization and internationalization support
+|
+|
+ ======================================================================"
+
+"======================================================================
+|
+| Copyright 2015 Free Software Foundation, Inc.
+| Written by Holger Hans Peter Freyther.
+|
+| This file is part of the GNU Smalltalk class library.
+|
+| The GNU Smalltalk class library is free software; you can redistribute it
+| and/or modify it under the terms of the GNU Lesser General Public License
+| as published by the Free Software Foundation; either version 2.1, or (at
+| your option) any later version.
+| 
+| The GNU Smalltalk class library is distributed in the hope that it will be
+| useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
+| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser
+| General Public License for more details.
+| 
+| You should have received a copy of the GNU Lesser General Public License
+| along with the GNU Smalltalk class library; see the file COPYING.LESSER.
+| If not, write to the Free Software Foundation, 59 Temple Place - Suite
+| 330, Boston, MA 02110-1301, USA.  
+|
+ ======================================================================"
+
+
+
+TestCase subclass: LcNumericTest [
+
+    testComputeNumberOfGroupsEmpty [
+        | numeric |
+        numeric := LcNumeric language: 'POSIX'.
+        numeric instVarNamed: #grouping put: #[]. 
+
+        self assert: (numeric computeNumberOfGroups: 0) equals: 0.
+        self assert: (numeric computeNumberOfGroups: 3) equals: 0.
+        self assert: (numeric computeNumberOfGroups: 6) equals: 0.
+        self assert: (numeric computeNumberOfGroups: 7) equals: 0.
+    ]
+
+    testComputeNumberOfGroupsThreeThree [
+        | numeric |
+        numeric := LcNumeric language: 'POSIX'.
+        numeric instVarNamed: #grouping put: #[3 3]. 
+
+        self assert: (numeric computeNumberOfGroups: 0) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 3) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 6) equals: 2.
+        self assert: (numeric computeNumberOfGroups: 7) equals: 3.
+        self assert: (numeric computeNumberOfGroups: 8) equals: 3.
+        self assert: (numeric computeNumberOfGroups: 9) equals: 3.
+        self assert: (numeric computeNumberOfGroups: 10) equals: 4.
+        self assert: (numeric computeNumberOfGroups: 12) equals: 4.
+        self assert: (numeric computeNumberOfGroups: 13) equals: 5.
+    ]
+
+    testComputeNumberOfGroupsThreeFixed [
+        | numeric |
+        numeric := LcNumeric language: 'POSIX'.
+        numeric instVarNamed: #grouping put: #[3 255]. 
+
+        self assert: (numeric computeNumberOfGroups: 0) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 3) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 6) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 7) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 9) equals: 1.
+        self assert: (numeric computeNumberOfGroups: 10) equals: 1.
+    ]
+
+    testEnglishLocale [
+        | numeric |
+        numeric := I18N.LcNumeric language: 'en_US'.
+
+        "Test with plain integers"
+        self assert: (numeric ? 123) equals: '123'.
+        self assert: (numeric ? 1234) equals: '1,234'.
+
+        "Test with float and digits"
+        self assert: (numeric ? 123.0) equals: '123.0'.
+        self assert: (numeric ? 1234.0) equals: '1,234.0'.
+        self assert: (numeric ? 12345.0) equals: '12,345.0'.
+        self assert: (numeric ? 123456.0) equals: '123,456.0'.
+        self assert: (numeric ? 1234567.0) equals: '1,234,567.0'.
+        self assert: (numeric ? 12345678.0) equals: '12,345,678.0'.
+        self assert: (numeric ? 123456789.0) equals: '123,456,789.0'.
+        self assert: (numeric ? 1234567891.0) equals: '1,234,567,891.0'.
+        self assert: (numeric ? 12345678912.0) equals: '12,345,678,912.0'.
+        self assert: (numeric ? 123456789123.0) equals: '123,456,789,123.0'.
+    ]
+
+    testPosixLocale [
+        | numeric |
+        numeric := I18N.LcNumeric language: 'POSIX'.
+
+        "Test with plain integers"
+        self assert: (numeric ? 123) equals: '123'.
+        self assert: (numeric ? 1234) equals: '1234'.
+
+        "Test with float and digits"
+        self assert: (numeric ? 123.0) equals: '123.0'.
+        self assert: (numeric ? 1234.0) equals: '1234.0'.
+        self assert: (numeric ? 12345.0) equals: '12345.0'.
+        self assert: (numeric ? 123456.0) equals: '123456.0'.
+        self assert: (numeric ? 1234567.0) equals: '1234567.0'.
+        self assert: (numeric ? 12345678.0) equals: '12345678.0'.
+        self assert: (numeric ? 123456789.0) equals: '123456789.0'.
+        self assert: (numeric ? 1234567891.0) equals: '1234567891.0'.
+        self assert: (numeric ? 12345678912.0) equals: '12345678912.0'.
+        self assert: (numeric ? 123456789123.0) equals: '123456789123.0'.
+    ]
+]
diff --git a/packages/i18n/Makefile.frag b/packages/i18n/Makefile.frag
index 5452584..9343428 100644
--- a/packages/i18n/Makefile.frag
+++ b/packages/i18n/Makefile.frag
@@ -1,5 +1,5 @@
 I18N_FILES = \
-packages/i18n/ChangeLog packages/i18n/Expression.st packages/i18n/GetText.st 
packages/i18n/Locale.st packages/i18n/Numbers.st packages/i18n/Times.st 
+packages/i18n/ChangeLog packages/i18n/Expression.st packages/i18n/GetText.st 
packages/i18n/LcNumericTest.st packages/i18n/Locale.st packages/i18n/Numbers.st 
packages/i18n/Times.st 
 $(I18N_FILES):
 $(srcdir)/packages/i18n/stamp-classes: $(I18N_FILES)
        touch $(srcdir)/packages/i18n/stamp-classes
diff --git a/packages/i18n/Numbers.st b/packages/i18n/Numbers.st
index d1db1ef..d4e3c50 100644
--- a/packages/i18n/Numbers.st
+++ b/packages/i18n/Numbers.st
@@ -65,7 +65,7 @@ a String according to the rules that are used in the given 
locale.'>
        decimal := nonLocalized indexOf: $. ifAbsent: [nonLocalized size + 1].
        stream := ReadWriteStream on: nonLocalized.
        stream
-           position: decimal;
+           position: decimal - 1;
            truncate;
            reset.
        self 
@@ -119,18 +119,41 @@ a String according to the rules that are used in the 
given locale.'>
 
     computeNumberOfGroups: size [
        <category: 'private'>
-       | left last n |
+       | left last n multiple |
        left := size.
        n := 0.
        grouping isEmpty ifTrue: [^0].
        grouping do: 
                [:each | 
-               n := n + 1.
+
+                "CHAR_MAX is an indicator for no more grouping
+                to be enabled. Stop here"
+                "No more grouping and repeating disabled"
                each >= 255 ifTrue: [^n].
-               left < each ifTrue: [^n].
+
+                "We have filled one more group"
+                n := n + 1.
+
+                "We consume the remaining characters and can
+                exit the loop now."
+                left <= each ifTrue: [^n].
+
+                "And consume it for the next iteration and
+                remember the last group size for repeating
+                it."
                left := left - each.
                last := each].
-       ^n + (left // last)
+
+        "We need to repeat grouping with the last size. This
+        is left / last and then adding one if there is a rest.
+        To avoid doing left // last twice and an extra addition
+        we remember the multiple and do parts of the modulo
+        operation here."
+        multiple := left // last.
+        n := n + multiple.
+        ^ (multiple * last) = left
+            ifTrue: [n]
+            ifFalse: [n + 1].
     ]
 
     printFractionalPart: string startingAt: decimal on: aStream [
diff --git a/packages/i18n/package.xml b/packages/i18n/package.xml
index 31980a7..89fd276 100644
--- a/packages/i18n/package.xml
+++ b/packages/i18n/package.xml
@@ -11,4 +11,9 @@
 
   <module>i18n</module>
   <file>ChangeLog</file>
+
+  <test>
+    <sunit>I18N.LcNumericTest</sunit>
+    <filein>LcNumericTest.st</filein>
+  </test>
 </package>
diff --git a/tests/testsuite.at b/tests/testsuite.at
index a23a41d..36d7888 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -157,6 +157,7 @@ AT_PACKAGE_TEST([DhbNumericalMethods])
 AT_PACKAGE_TEST([Digest])
 AT_OPTIONAL_PACKAGE_TEST([GDBM])
 AT_OPTIONAL_PACKAGE_TEST([Iconv])
+AT_OPTIONAL_PACKAGE_TEST([I18N])
 AT_PACKAGE_TEST([Kernel-Tests])
 AT_PACKAGE_TEST([Magritte])
 AT_OPTIONAL_PACKAGE_TEST([Netlink])
-- 
2.6.0