mirror of
https://github.com/qbittorrent/qBittorrent.git
synced 2024-11-21 16:55:46 +03:00
Improve natural sort algorithm
1. Use proper case folding function instead of `toLower()`. 2. Use locale aware comparison instead of comparing unicode code points. Now `a` comes before `A` which is the same as the result from QCollator. A nice side effect is now it properly compares locale specific characters (for example `C`, `Č`). 3. Improve testing. Now the test is runnable and stable on all platforms. PR #20208.
This commit is contained in:
Chocobo1
GitHub
parent
e69f857828
commit
5b3b56c918
3 changed files with 42 additions and 34 deletions
|
|
@ -31,7 +31,6 @@
|
|||
#include <QChar>
|
||||
#include <QString>
|
||||
|
||||
#ifndef QBT_USE_QCOLLATOR
|
||||
int Utils::Compare::naturalCompare(const QString &left, const QString &right, const Qt::CaseSensitivity caseSensitivity)
|
||||
{
|
||||
// Return value <0: `left` is smaller than `right`
|
||||
|
|
@ -45,8 +44,8 @@ int Utils::Compare::naturalCompare(const QString &left, const QString &right, co
|
|||
if ((posL == left.size()) || (posR == right.size()))
|
||||
return (left.size() - right.size()); // when a shorter string is another string's prefix, shorter string place before longer string
|
||||
|
||||
const QChar leftChar = (caseSensitivity == Qt::CaseSensitive) ? left[posL] : left[posL].toLower();
|
||||
const QChar rightChar = (caseSensitivity == Qt::CaseSensitive) ? right[posR] : right[posR].toLower();
|
||||
const QChar leftChar = (caseSensitivity == Qt::CaseSensitive) ? left[posL] : left[posL].toCaseFolded();
|
||||
const QChar rightChar = (caseSensitivity == Qt::CaseSensitive) ? right[posR] : right[posR].toCaseFolded();
|
||||
// Compare only non-digits.
|
||||
// Numbers should be compared as a whole
|
||||
// otherwise the string->int conversion can yield a wrong value
|
||||
|
|
@ -89,8 +88,7 @@ int Utils::Compare::naturalCompare(const QString &left, const QString &right, co
|
|||
}
|
||||
else
|
||||
{
|
||||
return (leftChar.unicode() - rightChar.unicode());
|
||||
return QString::localeAwareCompare(leftChar, rightChar);
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -34,24 +34,35 @@
|
|||
// for QT_FEATURE_xxx, see: https://wiki.qt.io/Qt5_Build_System#How_to
|
||||
#include <QtCore/private/qtcore-config_p.h>
|
||||
|
||||
#ifndef QBT_USE_QCOLLATOR
|
||||
// macOS and Windows support 'case sensitivity' and 'numeric mode' natively
|
||||
// https://github.com/qt/qtbase/blob/6.0/src/corelib/CMakeLists.txt#L777-L793
|
||||
// https://github.com/qt/qtbase/blob/6.0/src/corelib/text/qcollator_macx.cpp#L74-L77
|
||||
// https://github.com/qt/qtbase/blob/6.0/src/corelib/text/qcollator_win.cpp#L72-L78
|
||||
#if ((QT_FEATURE_icu == 1) || defined(Q_OS_MACOS) || defined(Q_OS_WIN))
|
||||
#define QBT_USE_QCOLLATOR
|
||||
#define QBT_USE_QCOLLATOR 1
|
||||
#include <QCollator>
|
||||
#else
|
||||
#define QBT_USE_QCOLLATOR 0
|
||||
#endif
|
||||
#endif
|
||||
|
||||
class QString;
|
||||
|
||||
namespace Utils::Compare
|
||||
{
|
||||
#ifdef QBT_USE_QCOLLATOR
|
||||
int naturalCompare(const QString &left, const QString &right, Qt::CaseSensitivity caseSensitivity);
|
||||
|
||||
template <Qt::CaseSensitivity caseSensitivity>
|
||||
class NaturalCompare
|
||||
{
|
||||
public:
|
||||
#if (QBT_USE_QCOLLATOR == 0)
|
||||
int operator()(const QString &left, const QString &right) const
|
||||
{
|
||||
return naturalCompare(left, right, caseSensitivity);
|
||||
}
|
||||
#else
|
||||
NaturalCompare()
|
||||
{
|
||||
m_collator.setNumericMode(true);
|
||||
|
|
@ -65,20 +76,8 @@ namespace Utils::Compare
|
|||
|
||||
private:
|
||||
QCollator m_collator;
|
||||
};
|
||||
#else
|
||||
int naturalCompare(const QString &left, const QString &right, Qt::CaseSensitivity caseSensitivity);
|
||||
|
||||
template <Qt::CaseSensitivity caseSensitivity>
|
||||
class NaturalCompare
|
||||
{
|
||||
public:
|
||||
int operator()(const QString &left, const QString &right) const
|
||||
{
|
||||
return naturalCompare(left, right, caseSensitivity);
|
||||
}
|
||||
};
|
||||
#endif
|
||||
};
|
||||
|
||||
template <Qt::CaseSensitivity caseSensitivity>
|
||||
class NaturalLessThan
|
||||
|
|
|
|||
|
|
@ -26,15 +26,16 @@
|
|||
* exception statement from your version.
|
||||
*/
|
||||
|
||||
#include <tuple>
|
||||
|
||||
#include <QLocale>
|
||||
#include <QObject>
|
||||
#include <QTest>
|
||||
|
||||
#include "base/global.h"
|
||||
|
||||
// only test qbt own implementation, not QCollator
|
||||
#define QBT_USE_QCOLLATOR 0
|
||||
#include "base/utils/compare.h"
|
||||
|
||||
#ifndef QBT_USE_QCOLLATOR // only test qbt own implementation, not QCollator
|
||||
namespace
|
||||
{
|
||||
enum class CompareResult
|
||||
|
|
@ -59,8 +60,8 @@ namespace
|
|||
{u"a"_s, u""_s, CompareResult::Greater, CompareResult::Greater},
|
||||
|
||||
{u"a"_s, u"a"_s, CompareResult::Equal, CompareResult::Equal},
|
||||
{u"A"_s, u"a"_s, CompareResult::Equal, CompareResult::Less}, // ascii code of 'A' is smaller than 'a'
|
||||
{u"a"_s, u"A"_s, CompareResult::Equal, CompareResult::Greater},
|
||||
{u"A"_s, u"a"_s, CompareResult::Equal, CompareResult::Greater},
|
||||
{u"a"_s, u"A"_s, CompareResult::Equal, CompareResult::Less},
|
||||
|
||||
{u"0"_s, u"0"_s, CompareResult::Equal, CompareResult::Equal},
|
||||
{u"1"_s, u"0"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
|
|
@ -71,17 +72,17 @@ namespace
|
|||
{u"😁"_s, u"😀"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
|
||||
{u"a1"_s, u"a1"_s, CompareResult::Equal, CompareResult::Equal},
|
||||
{u"A1"_s, u"a1"_s, CompareResult::Equal, CompareResult::Less},
|
||||
{u"a1"_s, u"A1"_s, CompareResult::Equal, CompareResult::Greater},
|
||||
{u"A1"_s, u"a1"_s, CompareResult::Equal, CompareResult::Greater},
|
||||
{u"a1"_s, u"A1"_s, CompareResult::Equal, CompareResult::Less},
|
||||
|
||||
{u"a1"_s, u"a2"_s, CompareResult::Less, CompareResult::Less},
|
||||
{u"A1"_s, u"a2"_s, CompareResult::Less, CompareResult::Less},
|
||||
{u"a1"_s, u"A2"_s, CompareResult::Less, CompareResult::Greater},
|
||||
{u"A1"_s, u"a2"_s, CompareResult::Less, CompareResult::Greater},
|
||||
{u"a1"_s, u"A2"_s, CompareResult::Less, CompareResult::Less},
|
||||
{u"A1"_s, u"A2"_s, CompareResult::Less, CompareResult::Less},
|
||||
|
||||
{u"abc100"_s, u"abc99"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
{u"ABC100"_s, u"abc99"_s, CompareResult::Greater, CompareResult::Less},
|
||||
{u"abc100"_s, u"ABC99"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
{u"ABC100"_s, u"abc99"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
{u"abc100"_s, u"ABC99"_s, CompareResult::Greater, CompareResult::Less},
|
||||
{u"ABC100"_s, u"ABC99"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
|
||||
{u"100abc"_s, u"99abc"_s, CompareResult::Greater, CompareResult::Greater},
|
||||
|
|
@ -135,7 +136,6 @@ namespace
|
|||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
class TestUtilsCompare final : public QObject
|
||||
{
|
||||
|
|
@ -145,8 +145,20 @@ class TestUtilsCompare final : public QObject
|
|||
public:
|
||||
TestUtilsCompare() = default;
|
||||
|
||||
#ifndef QBT_USE_QCOLLATOR // only test qbt own implementation, not QCollator
|
||||
private slots:
|
||||
void initTestCase() const
|
||||
{
|
||||
// Test will fail if ran with `C` locale. This is because `C` locale compare chars by code points
|
||||
// and doesn't take account of human expectations
|
||||
QLocale::setDefault(QLocale::English);
|
||||
}
|
||||
|
||||
void cleanupTestCase() const
|
||||
{
|
||||
// restore global state
|
||||
QLocale::setDefault(QLocale::system());
|
||||
}
|
||||
|
||||
void testNaturalCompareCaseInsensitive() const
|
||||
{
|
||||
const Utils::Compare::NaturalCompare<Qt::CaseInsensitive> cmp;
|
||||
|
|
@ -178,7 +190,6 @@ private slots:
|
|||
for (const TestData &data : testData)
|
||||
testLessThan(data, cmp(data.lhs, data.rhs), data.caseSensitiveResult);
|
||||
}
|
||||
#endif
|
||||
};
|
||||
|
||||
QTEST_APPLESS_MAIN(TestUtilsCompare)
|
||||
|
|
|
|||
Loading…
Reference in a new issue