Skip to content

Commit

Permalink
[mlocale] Avoid broken MLocale::localeScripts() code and adjust tests…
Browse files Browse the repository at this point in the history
…. JB#52793

The locale script data seems removed from ICU in 2015 so
we need a reimplementation if there's any need for the method.
The method could be beneficial if there's wider use for the library
so made it just warns instead of total removal. But at the moment
the usage is quite thin in Sailfish side.

On non-ICU build this was defining the method but didn't have
an implementation. Made the method consistently defined now.

Disabled some related testing in ft_locales.

On testing side, also disabled @pinyinsearch collation which
didn't work and for which I didn't find any definitions.
Using that in real code would be making quite specific expectations
as it's not included in any documentation.

kk_Cyrl_KZ existence test removed. Deleted from ICU in 2015 CLDR 28
integration.
  • Loading branch information
pvuorela committed Feb 14, 2022
1 parent 9609e0f commit ce5b4ca
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
14 changes: 11 additions & 3 deletions src/mlocale.cpp
Expand Up @@ -2858,10 +2858,12 @@ void MLocalePrivate::fixFormattedNumberForRTL(QString *formattedNumber) const
// make sure the result is not reordered again depending on
// context (this assumes that the formats are all edited exactly
// as they should appear in display order already!):
#if 0 // non-functional
if(q->localeScripts()[0] != "Arab" && q->localeScripts()[0] != "Hebr")
return;
formattedNumber->prepend(QChar(0x202A)); // LEFT-TO-RIGHT EMBEDDING
formattedNumber->append(QChar(0x202C)); // POP DIRECTIONAL FORMATTING
#endif
return;
}
#endif
Expand Down Expand Up @@ -3815,11 +3817,13 @@ QString MLocale::joinStringList(const QStringList &texts) const
{
QStringList textsWithBidiMarkers;
QString separator(QLatin1String(", "));
#if 0 // non-functional
if (localeScripts()[0] == QLatin1String("Arab"))
separator = QString::fromUtf8("، "); // U+060C ARABIC COMMA + space
else if (localeScripts().contains(QLatin1String("Hani"))) {
separator = QString::fromUtf8("、"); // U+3001 IDEOGRAPHIC COMMA, no space
}
#endif
foreach (const QString &text, texts) {
if (MLocale::directionForText(text) == Qt::RightToLeft)
// RIGHT-TO-LEFT EMBEDDING + text + POP DIRECTIONAL FORMATTING
Expand Down Expand Up @@ -4009,9 +4013,12 @@ QString MLocale::indexBucket(const QString &str) const
}
#endif

#ifdef HAVE_ICU
QStringList MLocale::localeScripts() const
{
QStringList scripts;
qWarning() << "MLocale::localeScripts() missing proper implementation. Add if needed.";
#if 0 // LocaleScript data removed from ICU, no point trying.
#ifdef HAVE_ICU
Q_D(const MLocale);
UErrorCode status = U_ZERO_ERROR;

Expand All @@ -4028,7 +4035,6 @@ QStringList MLocale::localeScripts() const
<< u_errorName(status);
}

QStringList scripts;
qint32 len;
const UChar *val;

Expand All @@ -4037,14 +4043,16 @@ QStringList MLocale::localeScripts() const
scripts << QString::fromUtf16(val, len);
}
ures_close(res);
#endif
#endif

if (scripts.isEmpty())
// "Zyyy" Code for undetermined script,
// see http://www.unicode.org/iso15924/iso15924-codes.html
scripts << "Zyyy";

return scripts;
}
#endif

void MLocale::copyCatalogsFrom(const MLocale &other)
{
Expand Down
12 changes: 11 additions & 1 deletion tests/ft_locales/ft_locales.cpp
Expand Up @@ -887,10 +887,14 @@ void Ft_Locales::testMLocaleLocaleScripts_data()

void Ft_Locales::testMLocaleLocaleScripts()
{
#if 0
QFETCH(QString, localeName);
QFETCH(QStringList, localeScripts);
MLocale locale(localeName);
QCOMPARE(locale.localeScripts(), localeScripts);
#else
QSKIP("LocaleScripts no longer in ICU data, thus MLocale::localeScripts() is non-functional. Skipping");
#endif
}

void Ft_Locales::testMLocaleToLower_data()
Expand Down Expand Up @@ -1255,6 +1259,9 @@ void Ft_Locales::testMLocaleJoinStringList_data()
<< (QStringList() << "a (b)" << "ش (ت)" << "c (d)" << "")
<< QString("‪a (b)‬, ‫ش (ت)‬, ‪c (d)‬, ‪中‬")
;

// MLocale::localeScript() needs to be reimplemented. Skipping locales that rely on that data.
return;
QTest::newRow("ar_EG")
<< QString("ar_EG")
<< (QStringList() << "a (b)" << "ش (ت)" << "c (d)" << "")
Expand Down Expand Up @@ -3951,6 +3958,8 @@ void Ft_Locales::testDifferentStrengthComparison_data()
QTest::addColumn<QString>("string2");
QTest::addColumn<QList<MLocale::Comparison> >("comparisonExpectedResults");

// not spotting anything for pinyinsearch in icu. Quite a special cased collation.
#ifdef ALSO_VERIFY_ICU_DOES_ITS_JOB_AS_WE_EXPECT
QTest::newRow("zh_CN@collation=pinyinsearch")
<<"ja_JP"
<<"zh_CN@collation=pinyinsearch"
Expand Down Expand Up @@ -4028,6 +4037,8 @@ void Ft_Locales::testDifferentStrengthComparison_data()
<< MLocale::LessThan
<< MLocale::LessThan
);
#endif

QTest::newRow("en_US")
<<"ja_JP"
<<"en_US"
Expand Down Expand Up @@ -4237,7 +4248,6 @@ void Ft_Locales::checkAvailableLocales()
requiredLocaleNames << "ja"; // "Japanese"
requiredLocaleNames << "ja_JP"; // "Japanese (Japan)"
requiredLocaleNames << "kk"; // "Kazakh"
requiredLocaleNames << "kk_Cyrl_KZ"; // "Kazakh (Kazakhstan)"
requiredLocaleNames << "lt"; // "Lithuanian"
requiredLocaleNames << "lt_LT"; // "Lithuanian (Lithuania)"
requiredLocaleNames << "km"; // "Khmer"
Expand Down

0 comments on commit ce5b4ca

Please sign in to comment.