AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix digit grouping: m_grouping_top doesn't mean what I thought it did

Browse Source 
I'd previously understood CLDR's minimumGroupingDigits to mean the
most significant group must have that many digits. It turns out to
mean only that the first grouping separator doesn't get added unless
the more significant group has this many. Once we have one separator,
more can be added that do isolate a single digit.

In the process, I discover some of the prior arithmetic is incorrect;
it is now fixed. Added some basic testing, amended some existing
tests. In the process, fixed naming of some double validator tests.

Pick-to: 6.6 6.5
Fixes: QTBUG-115740
Change-Id: Ia6ce011ba72e72428b015ca22b97d815ebf751b2
Reviewed-by: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
This commit is contained in:
Edward Welbourne 2023-08-04 16:52:41 +02:00
parent f493b72ee7
commit c5515f5eb1
5 changed files with 119 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/corelib/text/qlocale.cpp
View File

@ -3675,7 +3675,7 @@ QString QLocaleData::doubleToString(double d, int precision, DoubleForm form,
int bias = 2 + minExponentDigits;
// Decimal form may get grouping separators inserted:
if (groupDigits && decpt >= m_grouping_top + m_grouping_least)
bias -= (decpt - m_grouping_top - m_grouping_least) / m_grouping_higher + 1;
bias -= (decpt - m_grouping_least) / m_grouping_higher + 1;
// X = decpt - 1 needs two digits if decpt > 10:
if (decpt > 10 && minExponentDigits == 1)
++bias;
@ -3764,7 +3764,7 @@ QString QLocaleData::decimalForm(QString &&digits, int decpt, int precision,
qsizetype i = decpt - m_grouping_least;
if (i >= m_grouping_top) {
digits.insert(i * digitWidth, group);
while ((i -= m_grouping_higher) >= m_grouping_top)
while ((i -= m_grouping_higher) > 0)
digits.insert(i * digitWidth, group);
}
}
@ -3872,7 +3872,7 @@ QString QLocaleData::applyIntegerFormatting(QString &&numStr, bool negative, int
if (i >= m_grouping_top) {
numStr.insert(i * digitWidth, group);
++usedWidth;
while ((i -= m_grouping_higher) >= m_grouping_top) {
while ((i -= m_grouping_higher) > 0) {
numStr.insert(i * digitWidth, group);
++usedWidth;
}
@ -4191,7 +4191,7 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
if (last_separator_idx == -1) {
// Check distance from the beginning of the digits:
if (start_of_digits_idx == -1 || m_grouping_top > digitsInGroup
|| digitsInGroup >= m_grouping_higher + m_grouping_top) {
|| digitsInGroup >= m_grouping_least + m_grouping_top) {
return false;
}
} else {

2
src/corelib/text/qlocale_p.h
View File

@ -486,7 +486,7 @@ public:
quint8 m_first_day_of_week : 3;
quint8 m_weekend_start : 3;
quint8 m_weekend_end : 3;
quint8 m_grouping_top : 2; // Must have this many before the first grouping separator
quint8 m_grouping_top : 2; // Don't group until more significant group has this many digits.
quint8 m_grouping_higher : 3; // Number of digits between grouping separators
quint8 m_grouping_least : 3; // Number of digits after last grouping separator (before decimal).
};

90
tests/auto/corelib/text/qlocale/tst_qlocale.cpp
View File

@ -134,6 +134,8 @@ private slots:
void systemLocaleDayAndMonthNames();
#endif
void numberGrouping_data();
void numberGrouping();
void numberGroupingIndia();
void numberFormatChakma();
@ -1229,7 +1231,9 @@ void tst_QLocale::doubleToString_data()
QTest::newRow("de_DE 1245678900 g -") << QString("de_DE") << QString("1.245.678.900") << 12456789e2 << 'g' << shortest;
QTest::newRow("de_DE 12456789100 g -") << QString("de_DE") << QString("12.456.789.100") << 124567891e2 << 'g' << shortest;
QTest::newRow("de_DE 12456789000 g -") << QString("de_DE") << QString("1,2456789E+10") << 12456789e3 << 'g' << shortest;
QTest::newRow("de_DE 120000 g -") << QString("de_DE") << QString("120.000") << 12e4 << 'g' << shortest;
QTest::newRow("de_DE 12000 g -")
<< QString("de_DE") << QString("12.000") << 12e3 << 'g' << shortest;
// 12e4 has "120.000" and "1.2E+05" of equal length; which shortest picks is unspecified.
QTest::newRow("de_DE 1200000 g -") << QString("de_DE") << QString("1,2E+06") << 12e5 << 'g' << shortest;
QTest::newRow("de_DE 1000 g -") << QString("de_DE") << QString("1.000") << 1e3 << 'g' << shortest;
QTest::newRow("de_DE 10000 g -") << QString("de_DE") << QString("1E+04") << 1e4 << 'g' << shortest;
@ -4056,6 +4060,90 @@ void tst_QLocale::systemLocaleDayAndMonthNames()
#endif // QT_NO_SYSTEMLOCALE
void tst_QLocale::numberGrouping_data()
{
QTest::addColumn<QLocale>("locale");
QTest::addColumn<int>("number");
QTest::addColumn<QString>("string");
// Number options set here are expected to be default, but set for the
// avoidance of uncertainty or susceptibility to changed defaults.
QLocale c(QLocale::C); // English-style, without separators.
c.setNumberOptions(c.numberOptions() | QLocale::OmitGroupSeparator);
QTest::newRow("c:1") << c << 1 << u"1"_s;
QTest::newRow("c:12") << c << 12 << u"12"_s;
QTest::newRow("c:123") << c << 123 << u"123"_s;
QTest::newRow("c:1234") << c << 1234 << u"1234"_s;
QTest::newRow("c:12345") << c << 12345 << u"12345"_s;
QTest::newRow("c:123456") << c << 123456 << u"123456"_s;
QTest::newRow("c:1234567") << c << 1234567 << u"1234567"_s;
QTest::newRow("c:12345678") << c << 12345678 << u"12345678"_s;
QTest::newRow("c:123456789") << c << 123456789 << u"123456789"_s;
QTest::newRow("c:1234567890") << c << 1234567890 << u"1234567890"_s;
QLocale en(QLocale::English); // English-style, with separators:
en.setNumberOptions(en.numberOptions() & ~QLocale::OmitGroupSeparator);
QTest::newRow("en:1") << en << 1 << u"1"_s;
QTest::newRow("en:12") << en << 12 << u"12"_s;
QTest::newRow("en:123") << en << 123 << u"123"_s;
QTest::newRow("en:1,234") << en << 1234 << u"1,234"_s;
QTest::newRow("en:12,345") << en << 12345 << u"12,345"_s;
QTest::newRow("en:123,456") << en << 123456 << u"123,456"_s;
QTest::newRow("en:1,234,567") << en << 1234567 << u"1,234,567"_s;
QTest::newRow("en:12,345,678") << en << 12345678 << u"12,345,678"_s;
QTest::newRow("en:123,456,789") << en << 123456789 << u"123,456,789"_s;
QTest::newRow("en:1,234,567,890") << en << 1234567890 << u"1,234,567,890"_s;
QLocale es(QLocale::Spanish); // Spanish-style, with separators
es.setNumberOptions(es.numberOptions() & ~QLocale::OmitGroupSeparator);
QTest::newRow("es:1") << es << 1 << u"1"_s;
QTest::newRow("es:12") << es << 12 << u"12"_s;
QTest::newRow("es:123") << es << 123 << u"123"_s;
// First split doesn't happen unless first group has at least two digits:
QTest::newRow("es:1234") << es << 1234 << u"1234"_s;
QTest::newRow("es:12.345") << es << 12345 << u"12.345"_s;
QTest::newRow("es:123.456") << es << 123456 << u"123.456"_s;
// Later splits aren't limited to two digits (QTBUG-115740):
QTest::newRow("es:1.234.567") << es << 1234567 << u"1.234.567"_s;
QTest::newRow("es:12.345.678") << es << 12345678 << u"12.345.678"_s;
QTest::newRow("es:123.456.789") << es << 123456789 << u"123.456.789"_s;
QTest::newRow("es:1.234.567.890") << es << 1234567890 << u"1.234.567.890"_s;
QLocale hi(QLocale::Hindi, QLocale::India);
hi.setNumberOptions(hi.numberOptions() & ~QLocale::OmitGroupSeparator);
QTest::newRow("hi:1") << hi << 1 << u"1"_s;
QTest::newRow("hi:12") << hi << 12 << u"12"_s;
QTest::newRow("hi:123") << hi << 123 << u"123"_s;
QTest::newRow("hi:1,234") << hi << 1234 << u"1,234"_s;
QTest::newRow("hi:12,345") << hi << 12345 << u"12,345"_s;
QTest::newRow("hi:1,23,456") << hi << 123456 << u"1,23,456"_s;
QTest::newRow("hi:12,34,567") << hi << 1234567 << u"12,34,567"_s;
QTest::newRow("hi:1,23,45,678") << hi << 12345678 << u"1,23,45,678"_s;
QTest::newRow("hi:12,34,56,789") << hi << 123456789 << u"12,34,56,789"_s;
QTest::newRow("hi:1,23,45,67,890") << hi << 1234567890 << u"1,23,45,67,890"_s;
}
void tst_QLocale::numberGrouping()
{
QFETCH(const QLocale, locale);
QFETCH(const int, number);
QFETCH(const QString, string);
QCOMPARE(locale.toString(number), string);
QLocale sys = QLocale::system();
if (sys.language() == locale.language()
&& sys.script() == locale.script()
&& sys.territory() == locale.territory()) {
sys.setNumberOptions(locale.numberOptions());
QCOMPARE(sys.toString(number), string);
if (QLocale() == sys) { // This normally should be the case.
QCOMPARE(u"%L1"_s.arg(number), string);
QCOMPARE(u"%L1"_s.arg(double(number), 0, 'f', 0), string);
}
}
}
void tst_QLocale::numberGroupingIndia()
{
const QLocale indian(QLocale::Hindi, QLocale::India);

28
tests/auto/gui/util/qdoublevalidator/tst_qdoublevalidator.cpp
View File

@ -417,7 +417,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("C standard with invalid digit grouping")
<< "C" << QDoubleValidator::StandardNotation << -1 << "1,234,5.678"
<< "12345.678";
QTest::newRow("C standard with invalid number of decimals")
QTest::newRow("C standard with invalid group size")
<< "C" << QDoubleValidator::StandardNotation << 2 << "-12,34.678"
<< "-1234.68";
QTest::newRow("C standard truncate decimals")
@ -446,7 +446,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("C scientific with invalid digit grouping")
<< "C" << QDoubleValidator::ScientificNotation << -1 << "12,34.98765e2"
<< "1.23498765e+05";
QTest::newRow("C scientific with invalid number of decimals")
QTest::newRow("C scientific with invalid group size")
<< "C" << QDoubleValidator::ScientificNotation << 2 << "-12,34.98765e2"
<< "-1.23e+05";
QTest::newRow("C scientific truncate decimals")
@ -486,7 +486,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("en standard with invalid digit grouping")
<< "en" << QDoubleValidator::StandardNotation << -1 << "-1,234,5.678"
<< "-12,345.678";
QTest::newRow("en standard with invalid number of decimals")
QTest::newRow("en standard with invalid group size")
<< "en" << QDoubleValidator::StandardNotation << 2 << "12,34.678"
<< "1,234.68";
QTest::newRow("en standard no fractional part")
@ -502,7 +502,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("en scientific with invalid digit grouping")
<< "en" << QDoubleValidator::ScientificNotation << -1 << "-12,34.98765e2"
<< "-1.23498765E+05";
QTest::newRow("en scientific with invalid number of decimals")
QTest::newRow("en scientific with invalid group size")
<< "en" << QDoubleValidator::ScientificNotation << 2 << "12,34.98765e2"
<< "1.23E+05";
QTest::newRow("en scientific no fractional part")
@ -529,7 +529,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("de standard with invalid digit grouping")
<< "de" << QDoubleValidator::StandardNotation << -1 << "1.234.5,678"
<< "12.345,678";
QTest::newRow("de standard with invalid number of decimals")
QTest::newRow("de standard with invalid group size")
<< "de" << QDoubleValidator::StandardNotation << 2 << "-12.34,678"
<< "-1.234,68";
QTest::newRow("de standard no fractional part")
@ -544,7 +544,7 @@ void tst_QDoubleValidator::fixup_data()
QTest::newRow("de scientific with invalid digit grouping")
<< "de" << QDoubleValidator::ScientificNotation << -1 << "12.34,98765e2"
<< "1,23498765E+05";
QTest::newRow("de scientific with invalid number of decimals")
QTest::newRow("de scientific with invalid group size")
<< "de" << QDoubleValidator::ScientificNotation << 2 << "-12.34,98765e2"
<< "-1,23E+05";
QTest::newRow("de scientific no fractional part")
@ -560,6 +560,22 @@ void tst_QDoubleValidator::fixup_data()
<< "de" << QDoubleValidator::ScientificNotation << -1 << "-12.34"
<< "-1,234E+03";
// es locale uses ',' as decimal point and '.' as grouping separator.
// It doesn't apply grouping unless the the next-to-least significant group
// has more than one digit in it.
QTest::newRow("es standard no digit grouping")
<< "es" << QDoubleValidator::StandardNotation << -1 << "1234,567" << "1234,567";
QTest::newRow("es standard with digit grouping")
<< "es" << QDoubleValidator::StandardNotation << -1 << "-12.345,678" << "-12.345,678";
QTest::newRow("es standard with invalid group size")
<< "es" << QDoubleValidator::StandardNotation << -1 << "1.234.5,678" << "12.345,678";
QTest::newRow("es standard with invalid digit grouping")
<< "es" << QDoubleValidator::StandardNotation << 2 << "-1.234,678" << "-1234,68";
QTest::newRow("es standard big with invalid digit grouping")
<< "es" << QDoubleValidator::StandardNotation << 2 << "-1234.678,9" << "-1.234.678,9";
QTest::newRow("es standard no fractional part")
<< "es" << QDoubleValidator::StandardNotation << -1 << "12.34" << "1234";
// hi locale uses '.' as decimal point and ',' as grouping separator.
// The rightmost group is of three digits, all the others contain two
// digits.

6
tests/auto/gui/util/qintvalidator/tst_qintvalidator.cpp
View File

@ -308,9 +308,9 @@ void tst_QIntValidator::fixup_data()
// Normally the groups contain three digits, but the leftmost group should
// have at least two digits.
QTest::newRow("es no digit grouping 1000") << "es" << "1000" << "1000";
QTest::newRow("es no digit grouping 10000") << "es" << "10000" << "10.000";
QTest::newRow("es with digit grouping") << "es" << "1000.000" << "1000.000";
QTest::newRow("es invalid digit grouping") << "es" << "1.000.000" << "1000.000";
QTest::newRow("es with digit grouping 10000") << "es" << "10000" << "10.000";
QTest::newRow("es with digit grouping million") << "es" << "1.000.000" << "1.000.000";
QTest::newRow("es invalid digit grouping") << "es" << "1000.000" << "1.000.000";
}
QTEST_APPLESS_MAIN(tst_QIntValidator)