Merge pull request #3908 from Chocobo1/rm_nsort

Replace `naturalSort()` with `naturalCompare()`.

src/base/utils/string.cpp

@@ -27,160 +27,148 @@
  * exception statement from your version.
  */
 
-#include <QByteArray>
-#include <QString>
-#include <QLocale>
-#include <cmath>
 #include "string.h"
 
-QString Utils::String::fromStdString(const std::string &str)
-{
-    return QString::fromUtf8(str.c_str());
-}
+#include <cmath>
 
-std::string Utils::String::toStdString(const QString &str)
-{
-    QByteArray utf8 = str.toUtf8();
-    return std::string(utf8.constData(), utf8.length());
-}
+#include <QByteArray>
+#include <QtGlobal>
+#include <QLocale>
+#ifdef QBT_USES_QT5
+#include <QCollator>
+#endif
+#ifdef Q_OS_MAC
+#include <QThreadStorage>
+#endif
 
-// uses lessThan comparison
-bool Utils::String::naturalSort(const QString &left, const QString &right, bool &result)
+namespace
 {
-    // Return value indicates if functions was successful
-    // result argument will contain actual comparison result if function was successful
-    int posL = 0;
-    int posR = 0;
-    do {
-        forever {
-            if (posL == left.size() || posR == right.size())
-                return false; // No data
-
-            QChar leftChar = left.at(posL);
-            QChar rightChar = right.at(posR);
-            bool leftCharIsDigit = leftChar.isDigit();
-            bool rightCharIsDigit = rightChar.isDigit();
-            if (leftCharIsDigit != rightCharIsDigit)
-                return false; // Digit positions mismatch
-
-            if (leftCharIsDigit)
-                break; // Both are digit, break this loop and compare numbers
-
-            if (leftChar != rightChar)
-                return false; // Strings' subsets before digit do not match
-
-            ++posL;
-            ++posR;
+    class NaturalCompare
+    {
+    public:
+        explicit NaturalCompare(const bool caseSensitive = true)
+            : m_caseSensitive(caseSensitive)
+        {
+#ifdef QBT_USES_QT5
+#if defined(Q_OS_WIN)
+            // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7
+            if (QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7)
+                return;
+#endif
+            m_collator.setNumericMode(true);
+            m_collator.setCaseSensitivity(caseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive);
+#endif
         }
 
-        QString temp;
-        while (posL < left.size()) {
-            if (left.at(posL).isDigit())
-                temp += left.at(posL);
-            else
-                break;
-            posL++;
-        }
-        int numL = temp.toInt();
-        temp.clear();
-
-        while (posR < right.size()) {
-            if (right.at(posR).isDigit())
-                temp += right.at(posR);
-            else
-                break;
-            posR++;
+        bool operator()(const QString &left, const QString &right) const
+        {
+#ifdef QBT_USES_QT5
+#if defined(Q_OS_WIN)
+            // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7
+            if (QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7)
+                return lessThan(left, right);
+#endif
+            return (m_collator.compare(left, right) < 0);
+#else
+            return lessThan(left, right);
+#endif
         }
-        int numR = temp.toInt();
 
-        if (numL != numR) {
-            result = (numL < numR);
-            return true;
-        }
+        bool lessThan(const QString &left, const QString &right) const
+        {
+            // Return value `false` indicates `right` should go before `left`, otherwise, after
+            int posL = 0;
+            int posR = 0;
+            while (true) {
+                while (true) {
+                    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
+
+                    QChar leftChar = m_caseSensitive ? left[posL] : left[posL].toLower();
+                    QChar rightChar = m_caseSensitive ? right[posR] : right[posR].toLower();
+                    if (leftChar == rightChar)
+                        ;  // compare next character
+                    else if (leftChar.isDigit() && rightChar.isDigit())
+                        break; // Both are digits, break this loop and compare numbers
+                    else
+                        return leftChar < rightChar;
+
+                    ++posL;
+                    ++posR;
+                }
+
+                int startL = posL;
+                while ((posL < left.size()) && left[posL].isDigit())
+                    ++posL;
+#ifdef QBT_USES_QT5
+                int numL = left.midRef(startL, posL - startL).toInt();
+#else
+                int numL = left.mid(startL, posL - startL).toInt();
+#endif
 
-        // Strings + digits do match and we haven't hit string end
-        // Do another round
+                int startR = posR;
+                while ((posR < right.size()) && right[posR].isDigit())
+                    ++posR;
+#ifdef QBT_USES_QT5
+                int numR = right.midRef(startR, posR - startR).toInt();
+#else
+                int numR = right.mid(startR, posR - startR).toInt();
+#endif
 
-    } while (true);
+                if (numL != numR)
+                    return (numL < numR);
 
-    return false;
-}
+                // Strings + digits do match and we haven't hit string end
+                // Do another round
+            }
+            return false;
+        }
 
-Utils::String::NaturalCompare::NaturalCompare()
-{
-#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0))
-#if defined(Q_OS_WIN)
-    // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7
-    if(QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7)
-        return;
-#endif
-    m_collator.setNumericMode(true);
-    m_collator.setCaseSensitivity(Qt::CaseInsensitive);
+    private:
+#ifdef QBT_USES_QT5
+        QCollator m_collator;
 #endif
+        const bool m_caseSensitive;
+    };
 }
 
-bool Utils::String::NaturalCompare::operator()(const QString &l, const QString &r)
+bool Utils::String::naturalCompareCaseSensitive(const QString &left, const QString &right)
 {
-#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0))
-#if defined(Q_OS_WIN)
-    // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7
-    if(QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7)
-        return lessThan(l, r);
-#endif
-    return (m_collator.compare(l, r) < 0);
+    // provide a single `NaturalCompare` instance for easy use
+    // https://doc.qt.io/qt-5/threads-reentrancy.html
+#ifdef Q_OS_MAC  // workaround for Apple xcode: https://stackoverflow.com/a/29929949
+    static QThreadStorage<NaturalCompare> nCmp;
+    if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(true));
+    return (nCmp.localData())(left, right);
 #else
-    return lessThan(l, r);
+    thread_local NaturalCompare nCmp(true);
+    return nCmp(left, right);
 #endif
 }
 
-bool Utils::String::NaturalCompare::lessThan(const QString &left, const QString &right)
+bool Utils::String::naturalCompareCaseInsensitive(const QString &left, const QString &right)
 {
-    // Return value `false` indicates `right` should go before `left`, otherwise, after
-    int posL = 0;
-    int posR = 0;
-    while (true) {
-        while (true) {
-            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
-
-            QChar leftChar = left[posL].toLower();
-            QChar rightChar = right[posR].toLower();
-            if (leftChar == rightChar)
-                ;  // compare next character
-            else if (leftChar.isDigit() && rightChar.isDigit())
-                break; // Both are digits, break this loop and compare numbers
-            else
-                return leftChar < rightChar;
-
-            ++posL;
-            ++posR;
-        }
-
-        int startL = posL;
-        while ((posL < left.size()) && left[posL].isDigit())
-            ++posL;
-#if QT_VERSION >= QT_VERSION_CHECK(5, 1, 0)
-        int numL = left.midRef(startL, posL - startL).toInt();
+    // provide a single `NaturalCompare` instance for easy use
+    // https://doc.qt.io/qt-5/threads-reentrancy.html
+#ifdef Q_OS_MAC  // workaround for Apple xcode: https://stackoverflow.com/a/29929949
+    static QThreadStorage<NaturalCompare> nCmp;
+    if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(false));
+    return (nCmp.localData())(left, right);
 #else
-        int numL = left.mid(startL, posL - startL).toInt();
-#endif
-
-        int startR = posR;
-        while ((posR < right.size()) && right[posR].isDigit())
-            ++posR;
-#if QT_VERSION >= QT_VERSION_CHECK(5, 1, 0)
-        int numR = right.midRef(startR, posR - startR).toInt();
-#else
-        int numR = right.mid(startR, posR - startR).toInt();
+    thread_local NaturalCompare nCmp(false);
+    return nCmp(left, right);
 #endif
+}
 
-        if (numL != numR)
-            return (numL < numR);
+QString Utils::String::fromStdString(const std::string &str)
+{
+    return QString::fromUtf8(str.c_str());
+}
 
-        // Strings + digits do match and we haven't hit string end
-        // Do another round
-    }
-    return false;
+std::string Utils::String::toStdString(const QString &str)
+{
+    QByteArray utf8 = str.toUtf8();
+    return std::string(utf8.constData(), utf8.length());
 }
 
 // to send numbers instead of strings with suffixes

src/base/utils/string.h

@@ -31,13 +31,9 @@
 #define UTILS_STRING_H
 
 #include <string>
-#include <QtGlobal>
-#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0))
-#include <QCollator>
-#endif
 
-class QString;
 class QByteArray;
+class QString;
 
 namespace Utils
 {
@@ -51,19 +47,8 @@ namespace Utils
         // Taken from https://crackstation.net/hashing-security.htm
         bool slowEquals(const QByteArray &a, const QByteArray &b);
 
-        bool naturalSort(const QString &left, const QString &right, bool &result);
-
-        class NaturalCompare
-        {
-        public:
-            NaturalCompare();
-            bool operator()(const QString &l, const QString &r);
-            bool lessThan(const QString &left, const QString &right);
-#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0))
-        private:
-            QCollator m_collator;
-#endif
-        };
+        bool naturalCompareCaseSensitive(const QString &left, const QString &right);
+        bool naturalCompareCaseInsensitive(const QString &left, const QString &right);
     }
 }
 

src/gui/addnewtorrentdialog.cpp

@@ -96,7 +96,7 @@ AddNewTorrentDialog::AddNewTorrentDialog(QWidget *parent)
 
     // Load categories
     QStringList categories = session->categories();
-    std::sort(categories.begin(), categories.end(), Utils::String::NaturalCompare());
+    std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive);
     QString defaultCategory = settings()->loadValue(KEY_DEFAULTCATEGORY).toString();
 
     if (!defaultCategory.isEmpty())

src/gui/properties/peerlistsortmodel.h

@@ -43,21 +43,16 @@ public:
 
 protected:
   bool lessThan(const QModelIndex &left, const QModelIndex &right) const {
-    if (sortColumn() == PeerListDelegate::IP || sortColumn() == PeerListDelegate::CLIENT) {
-      QVariant vL = sourceModel()->data(left);
-      QVariant vR = sourceModel()->data(right);
-      if (!(vL.isValid() && vR.isValid()))
-        return QSortFilterProxyModel::lessThan(left, right);
-      Q_ASSERT(vL.isValid());
-      Q_ASSERT(vR.isValid());
-
-      bool res = false;
-      if (Utils::String::naturalSort(vL.toString(), vR.toString(), res))
-        return res;
+  switch (sortColumn()) {
+  case PeerListDelegate::IP:
+  case PeerListDelegate::CLIENT: {
+    QString vL = left.data().toString();
+    QString vR = right.data().toString();
+    return Utils::String::naturalCompareCaseSensitive(vL, vR);
+  }
+  };
 
-      return QSortFilterProxyModel::lessThan(left, right);
-    }
-    return QSortFilterProxyModel::lessThan(left, right);
+  return QSortFilterProxyModel::lessThan(left, right);
   }
 };
 

src/gui/rss/automatedrssdownloader.cpp

@@ -314,7 +314,7 @@ void AutomatedRssDownloader::initCategoryCombobox()
 {
   // Load torrent categories
   QStringList categories = BitTorrent::Session::instance()->categories();
-  std::sort(categories.begin(), categories.end(), Utils::String::NaturalCompare());
+  std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive);
   ui->comboCategory->addItems(categories);
 }
 

src/gui/search/searchsortmodel.cpp

@@ -107,20 +107,17 @@ qint64 SearchSortModel::maxSize() const
 
 bool SearchSortModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
 {
-    if ((sortColumn() == NAME) || (sortColumn() == ENGINE_URL)) {
-        QVariant vL = sourceModel()->data(left);
-        QVariant vR = sourceModel()->data(right);
-        if (!(vL.isValid() && vR.isValid()))
-            return base::lessThan(left, right);
-        Q_ASSERT(vL.isValid());
-        Q_ASSERT(vR.isValid());
-
-        bool res = false;
-        if (Utils::String::naturalSort(vL.toString(), vR.toString(), res))
-            return res;
+    switch (sortColumn()) {
+    case NAME:
+    case ENGINE_URL: {
+        QString vL = left.data().toString();
+        QString vR = right.data().toString();
+        return Utils::String::naturalCompareCaseSensitive(vL, vR);
     }
 
-    return base::lessThan(left, right);
+    default:
+        return base::lessThan(left, right);
+    };
 }
 
 bool SearchSortModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const

src/gui/torrentcontentfiltermodel.cpp

@@ -82,28 +82,22 @@ bool TorrentContentFilterModel::filterAcceptsRow(int source_row, const QModelInd
 }
 
 bool TorrentContentFilterModel::lessThan(const QModelIndex &left, const QModelIndex &right) const {
-  if (sortColumn() == NAME) {
-    QVariant vL = sourceModel()->data(left);
-    QVariant vR = sourceModel()->data(right);
-    if (!(vL.isValid() && vR.isValid()))
-      return QSortFilterProxyModel::lessThan(left, right);
-    Q_ASSERT(vL.isValid());
-    Q_ASSERT(vR.isValid());
+  switch (sortColumn()) {
+  case NAME: {  // PropColumn::NAME
+    QString vL = left.data().toString();
+    QString vR = right.data().toString();
+    TorrentContentModelItem::ItemType leftType = m_model->itemType(m_model->index(left.row(), 0, left.parent()));
+    TorrentContentModelItem::ItemType rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent()));
 
-    TorrentContentModelItem::ItemType leftType, rightType;
-    leftType = m_model->itemType(m_model->index(left.row(), 0, left.parent()));
-    rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent()));
-    if (leftType == rightType) {
-      bool res = false;
-      if (Utils::String::naturalSort(vL.toString(), vR.toString(), res))
-        return res;
-      return QSortFilterProxyModel::lessThan(left, right);
-    }
+    if (leftType == rightType)
+      return Utils::String::naturalCompareCaseSensitive(vL, vR);
     else if (leftType == TorrentContentModelItem::FolderType && sortOrder() == Qt::AscendingOrder)
       return true;
     else
-       return false;
+      return false;
   }
+  };
+
   return QSortFilterProxyModel::lessThan(left, right);
 }
 

src/gui/transferlistfilterswidget.cpp

@@ -237,17 +237,14 @@ void CategoryFiltersList::addItem(const QString &category, bool hasTorrent)
     if (exists) return;
 
     Q_ASSERT(count() >= 2);
+    int insPos = count();
     for (int i = 2; i < count(); ++i) {
-        bool less = false;
-        if (!(Utils::String::naturalSort(category, item(i)->text(), less)))
-            less = (category.localeAwareCompare(item(i)->text()) < 0);
-        if (less) {
-            insertItem(i, categoryItem);
-            updateGeometry();
-            return;
+        if (Utils::String::naturalCompareCaseSensitive(category, item(i)->text())) {
+            insPos = i;
+            break;
         }
     }
-    QListWidget::addItem(categoryItem);
+    QListWidget::insertItem(insPos, categoryItem);
     updateGeometry();
 }
 
@@ -512,17 +509,14 @@ void TrackerFiltersList::addItem(const QString &tracker, const QString &hash)
     }
 
     Q_ASSERT(count() >= 4);
-    for (int i = 4; i<count(); ++i) {
-        bool less = false;
-        if (!(Utils::String::naturalSort(host, item(i)->text(), less)))
-            less = (host.localeAwareCompare(item(i)->text()) < 0);
-        if (less) {
-            insertItem(i, trackerItem);
-            updateGeometry();
-            return;
+    int insPos = count();
+    for (int i = 4; i < count(); ++i) {
+        if (Utils::String::naturalCompareCaseSensitive(host, item(i)->text())) {
+            insPos = i;
+            break;
         }
     }
-    QListWidget::addItem(trackerItem);
+    QListWidget::insertItem(insPos, trackerItem);
     updateGeometry();
 }
 

src/gui/transferlistsortmodel.cpp

@@ -73,21 +73,19 @@ void TransferListSortModel::disableTrackerFilter()
 
 bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
 {
-    const int column  = sortColumn();
-
-    if (column == TorrentModel::TR_NAME) {
+    switch (sortColumn()) {
+    case TorrentModel::TR_NAME: {
         QVariant vL = left.data();
         QVariant vR = right.data();
         if (!vL.isValid() || !vR.isValid() || (vL == vR))
             return lowerPositionThan(left, right);
 
-        bool res = false;
-        if (Utils::String::naturalSort(vL.toString(), vR.toString(), res))
-            return res;
-
-        return QSortFilterProxyModel::lessThan(left, right);
+        return Utils::String::naturalCompareCaseSensitive(vL.toString(), vR.toString());
     }
-    else if (column == TorrentModel::TR_ADD_DATE || column == TorrentModel::TR_SEED_DATE || column == TorrentModel::TR_SEEN_COMPLETE_DATE) {
+
+    case TorrentModel::TR_ADD_DATE:
+    case TorrentModel::TR_SEED_DATE:
+    case TorrentModel::TR_SEEN_COMPLETE_DATE: {
         QDateTime vL = left.data().toDateTime();
         QDateTime vR = right.data().toDateTime();
 
@@ -97,10 +95,13 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
 
         return vL < vR;
     }
-    else if (column == TorrentModel::TR_PRIORITY) {
+
+    case TorrentModel::TR_PRIORITY: {
         return lowerPositionThan(left, right);
     }
-    else if (column == TorrentModel::TR_PEERS || column == TorrentModel::TR_SEEDS) {
+
+    case TorrentModel::TR_SEEDS:
+    case TorrentModel::TR_PEERS: {
         int left_active = left.data().toInt();
         int left_total = left.data(Qt::UserRole).toInt();
         int right_active = right.data().toInt();
@@ -116,7 +117,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
             return (left_active < right_active);
         }
     }
-    else if (column == TorrentModel::TR_ETA) {
+
+    case TorrentModel::TR_ETA: {
         TorrentModel *model = qobject_cast<TorrentModel *>(sourceModel());
         const int prioL = model->data(model->index(left.row(), TorrentModel::TR_PRIORITY)).toInt();
         const int prioR = model->data(model->index(right.row(), TorrentModel::TR_PRIORITY)).toInt();
@@ -164,7 +166,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
             return !invalidL;
         }
     }
-    else if (column == TorrentModel::TR_LAST_ACTIVITY) {
+
+    case TorrentModel::TR_LAST_ACTIVITY: {
         const qlonglong vL = left.data().toLongLong();
         const qlonglong vR = right.data().toLongLong();
 
@@ -173,7 +176,8 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
 
         return vL < vR;
     }
-    else if (column == TorrentModel::TR_RATIO_LIMIT) {
+
+    case TorrentModel::TR_RATIO_LIMIT: {
         const qreal vL = left.data().toDouble();
         const qreal vR = right.data().toDouble();
 
@@ -183,10 +187,12 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
         return vL < vR;
     }
 
-    if (left.data() == right.data())
-        return lowerPositionThan(left, right);
-
-    return QSortFilterProxyModel::lessThan(left, right);
+    default: {
+        if (left.data() == right.data())
+            return lowerPositionThan(left, right);
+        return QSortFilterProxyModel::lessThan(left, right);
+    }
+    };
 }
 
 bool TransferListSortModel::lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const

src/gui/transferlistwidget.cpp

@@ -769,7 +769,7 @@ void TransferListWidget::displayListMenu(const QPoint&)
         listMenu.addAction(&actionRename);
     // Category Menu
     QStringList categories = BitTorrent::Session::instance()->categories();
-    std::sort(categories.begin(), categories.end(), Utils::String::NaturalCompare());
+    std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive);
     QList<QAction*> categoryActions;
     QMenu *categoryMenu = listMenu.addMenu(GuiIconProvider::instance()->getIcon("view-categories"), tr("Category"));
     categoryActions << categoryMenu->addAction(GuiIconProvider::instance()->getIcon("list-add"), tr("New...", "New category..."));