[PATCH v6 0/1] MR1937: kernelbase: Fix grouping repeat for number formatting.
LOCALE_SGROUPING's string and the `Grouping` field in NUMBERFMTW are confusingly different. The former, which is what is fixed here, treats a '0' as a repeat of the previous grouping. But a 0 at the end of the `Grouping` field prevents it from repeating (it repeats by default otherwise) so it's the opposite. Note that without a '0' in the LOCALE_SGROUPING string, it shouldn't even repeat in the first place. This fixes the typical "3;0" default grouping, for example. See: https://learn.microsoft.com/en-us/windows/win32/intl/locale-sgrouping -- v6: kernelbase: Fix grouping repeat for number formatting. https://gitlab.winehq.org/wine/wine/-/merge_requests/1937
Ok, I found another corner case (multiple trailing zeros in the grouping string) which seems a bit weird. I was hoping to get rid of the special case in `grouping_to_string` but unfortunately it's still needed (although less hacky, doesn't end in semi-colon anymore). This is probably the best I can do now while still keeping it correct… Sorry for the noise. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_21220
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Fixes a regression introduced by 56099a31242dcc522fb94643e70553e152c522ec. LOCALE_SGROUPING's string and the `Grouping` field in NUMBERFMTW are confusingly different. The former, which is what is fixed here, treats a '0' as a repeat of the previous grouping. But a 0 at the end of the `Grouping` field prevents it from repeating (it repeats by default otherwise) so it's the opposite. Note that without a '0' in the LOCALE_SGROUPING string, it shouldn't even repeat in the first place. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/kernel32/tests/locale.c | 78 ++++++++++++++++++++++++++++++++++++ dlls/kernelbase/locale.c | 66 +++++++++++++++++++++++++++--- 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index 886240cc22c..b2c581dc518 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -1404,6 +1404,7 @@ static void test_GetNumberFormatA(void) static char szComma[] = { ',', '\0' }; int ret; LCID lcid = MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), SORT_DEFAULT); + WCHAR grouping[32], t1000[8], dec[8], frac[8], lzero[8]; char buffer[BUFFER_SIZE]; NUMBERFMTA format; @@ -1522,6 +1523,22 @@ static void test_GetNumberFormatA(void) ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer)); expect_str(ret, buffer, "1234,567,89.0"); + format.Grouping = 203; + ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer)); + expect_str(ret, buffer, "1,234,567,,89.0"); + + format.Grouping = 2030; + ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer)); + expect_str(ret, buffer, "1234,567,,89.0"); + + format.Grouping = 2003; + ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer)); + expect_str(ret, buffer, "1,234,567,,,89.0"); + + format.Grouping = 1200; + ret = GetNumberFormatA(lcid, 0, "123456789", &format, buffer, ARRAY_SIZE(buffer)); + expect_str(ret, buffer, "123456,,78,9.0"); + /* Grouping of a negative number */ format.NegativeOrder = NEG_LEFT; format.Grouping = 3; @@ -1563,6 +1580,67 @@ static void test_GetNumberFormatA(void) ret = GetNumberFormatA(lcid, NUO, "-12345", NULL, buffer, ARRAY_SIZE(buffer)); expect_str(ret, buffer, "-12\xa0\x33\x34\x35,00"); /* Non breaking space */ } + + /* Test the actual LOCALE_SGROUPING string, the rules for repeats are opposite */ + if (GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, grouping, ARRAY_SIZE(grouping)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, t1000, ARRAY_SIZE(t1000)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, dec, ARRAY_SIZE(dec)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, frac, ARRAY_SIZE(frac)) && + GetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_ILZERO, lzero, ARRAY_SIZE(lzero))) + { + static const struct + { + const char *grouping; + const char *expected; + } tests[] = { + { "3;0", "1,234,567,890.54321" }, + { "2;3", "12345,678,90.54321" }, + { "1", "123456789,0.54321" }, + { "1;0", "1,2,3,4,5,6,7,8,9,0.54321" }, + { "1;0;3", "123456,789,,0.54321" }, + { "0", "1234567890.54321" }, + { "0;0", "1234567890.54321" }, + { "0;1", "123456789,0.54321" }, + { "0;0;0", "1234567890.54321" }, + { "0;1;0", "1,2,3,4,5,6,7,8,9,0.54321" }, + { "2;0;0", "12345678,90.54321" }, + { "2;0;0;0", "12345678,,90.54321" }, + { "2;0;0;0;0", "12345678,,,90.54321" }, + { "2;0;0;1;0", "1,2,3,4,5,6,7,8,,,90.54321" }, + { "1;3;2", "1234,56,789,0.54321" }, + { "1;3;2;0", "12,34,56,789,0.54321" }, + { "3;1;1;2;0", "1,23,45,6,7,890.54321" }, + { "6;1", "123,4,567890.54321" }, + }; + unsigned i; + + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, ","); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, "."); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, "5"); + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_ILZERO, "0"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) + { + SetLocaleInfoA(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, tests[i].grouping); + SetLastError(0xdeadbeef); + ret = GetNumberFormatA(LOCALE_USER_DEFAULT, 0, "1234567890.54321", NULL, buffer, ARRAY_SIZE(buffer)); + if (ret) + { + ok(GetLastError() == 0xdeadbeef, "[%u] unexpected error %lu\n", i, GetLastError()); + ok(ret == strlen(tests[i].expected) + 1, "[%u] unexpected ret %d\n", i, ret); + ok(!strcmp(buffer, tests[i].expected), "[%u] unexpected string %s\n", i, buffer); + } + else + ok(0, "[%u] expected success, got error %ld\n", i, GetLastError()); + } + + /* Restore */ + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SGROUPING, grouping), "Restoring SGROUPING failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, t1000), "Restoring STHOUSAND failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, dec), "Restoring SDECIMAL failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_IDIGITS, frac), "Restoring IDIGITS failed\n"); + ok(SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_ILZERO, lzero), "Restoring ILZERO failed\n"); + } } static void test_GetNumberFormatEx(void) diff --git a/dlls/kernelbase/locale.c b/dlls/kernelbase/locale.c index 1e96e49622e..13ec1ebeb3a 100644 --- a/dlls/kernelbase/locale.c +++ b/dlls/kernelbase/locale.c @@ -7245,8 +7245,20 @@ BOOL WINAPI SetUserGeoName(PWSTR geo_name) static void grouping_to_string( UINT grouping, WCHAR *buffer ) { + UINT last_digit = grouping % 10; WCHAR tmp[10], *p = tmp; + /* The string is confusingly different when it comes to repetitions (trailing zeros). For a string, + * a 0 signals that the format needs to be repeated, which is the opposite of the grouping integer. */ + if (last_digit == 0) + { + grouping /= 10; + + /* Special case: two or more trailing zeros result in zero-sided groupings, with no repeats */ + if (grouping % 10 == 0) + last_digit = ~0; + } + while (grouping) { *p++ = '0' + grouping % 10; @@ -7257,6 +7269,17 @@ static void grouping_to_string( UINT grouping, WCHAR *buffer ) *buffer++ = *(--p); if (p > tmp) *buffer++ = ';'; } + if (last_digit != 0) + { + *buffer++ = ';'; + *buffer++ = '0'; + if (last_digit == ~0) + { + /* Add another trailing zero due to the weird way trailing zeros work in grouping string */ + *buffer++ = ';'; + *buffer++ = '0'; + } + } *buffer = 0; } @@ -7272,9 +7295,9 @@ static WCHAR *prepend_str( WCHAR *end, const WCHAR *str ) static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decimal_sep, const WCHAR *thousand_sep, const WCHAR *grouping, UINT digits, BOOL lzero ) { + BOOL round = FALSE, repeat = FALSE; + UINT i, len = 0, prev = ~0; const WCHAR *frac = NULL; - BOOL round = FALSE; - UINT i, len = 0; *(--end) = 0; @@ -7327,9 +7350,43 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima } if (len) lzero = FALSE; + /* leading 0s are ignored */ + while (grouping[0] == '0' && grouping[1] == ';') + grouping += 2; + while (len) { - UINT limit = *grouping == '0' ? ~0u : *grouping - '0'; + UINT limit = prev; + + if (!repeat) + { + limit = *grouping - '0'; + if (grouping[1] == ';') + { + grouping += 2; + if (limit) + prev = limit; + else + { + /* Trailing 0;0 is a special case */ + prev = ~0; + if (grouping[0] == '0' && grouping[1] != ';') + { + repeat = TRUE; + limit = prev; + } + } + } + else + { + repeat = TRUE; + if (!limit) + limit = prev; + else + prev = ~0; + } + } + while (len && limit--) { WCHAR ch = value[--len]; @@ -7345,7 +7402,6 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima *(--end) = ch; } if (len) end = prepend_str( end, thousand_sep ); - if (grouping[1] == ';') grouping += 2; } if (round) *(--end) = '1'; else if (lzero) *(--end) = '0'; @@ -7356,7 +7412,7 @@ static WCHAR *format_number( WCHAR *end, const WCHAR *value, const WCHAR *decima static int get_number_format( const NLS_LOCALE_DATA *locale, DWORD flags, const WCHAR *value, const NUMBERFMTW *format, WCHAR *buffer, int len ) { - WCHAR *num, fmt_decimal[4], fmt_thousand[4], fmt_neg[5], grouping[20], output[256]; + WCHAR *num, fmt_decimal[4], fmt_thousand[4], fmt_neg[5], grouping[24], output[256]; const WCHAR *decimal_sep = fmt_decimal, *thousand_sep = fmt_thousand; DWORD digits, lzero, order; int ret = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1937
Hi, is there something I should do to improve this, handle it differently, or just rebase it? Now that code freeze is over, and it's still a regression… -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1937#note_23120
participants (1)
-
Gabriel Ivăncescu