diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp index 302186753..a7a58edf5 100644 --- a/src/base/utils/string.cpp +++ b/src/base/utils/string.cpp @@ -33,9 +33,9 @@ #include #include -#include #include #include +#include #ifdef Q_OS_MAC #include #endif @@ -45,110 +45,103 @@ namespace class NaturalCompare { public: - explicit NaturalCompare(const bool caseSensitive = true) - : m_caseSensitive(caseSensitive) + explicit NaturalCompare(const Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive) + : m_caseSensitivity(caseSensitivity) { -#if defined(Q_OS_WIN) +#ifdef Q_OS_WIN // Without ICU library, QCollator uses the native API on Windows 7+. But that API // sorts older versions of μTorrent differently than the newer ones because the // 'μ' character is encoded differently and the native API can't cope with that. // So default to using our custom natural sorting algorithm instead. // See #5238 and #5240 - // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7 - // if (QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7) - return; -#endif + // Without ICU library, QCollator doesn't support `setNumericMode(true)` on an OS older than Win7 +#else m_collator.setNumericMode(true); - m_collator.setCaseSensitivity(caseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive); - } - - bool operator()(const QString &left, const QString &right) const - { -#if defined(Q_OS_WIN) - // Without ICU library, QCollator uses the native API on Windows 7+. But that API - // sorts older versions of μTorrent differently than the newer ones because the - // 'μ' character is encoded differently and the native API can't cope with that. - // So default to using our custom natural sorting algorithm instead. - // See #5238 and #5240 - // Without ICU library, QCollator doesn't support `setNumericMode(true)` on OS older than Win7 - // if (QSysInfo::windowsVersion() < QSysInfo::WV_WINDOWS7) - return lessThan(left, right); + m_collator.setCaseSensitivity(caseSensitivity); #endif - return (m_collator.compare(left, right) < 0); } - bool lessThan(const QString &left, const QString &right) const + int operator()(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; - int numL = left.midRef(startL, posL - startL).toInt(); - - int startR = posR; - while ((posR < right.size()) && right[posR].isDigit()) - ++posR; - int numR = right.midRef(startR, posR - startR).toInt(); - - if (numL != numR) - return (numL < numR); - - // Strings + digits do match and we haven't hit string end - // Do another round - } - return false; +#ifdef Q_OS_WIN + return compare(left, right); +#else + return m_collator.compare(left, right); +#endif } private: + int compare(const QString &left, const QString &right) const + { + // Return value <0: `left` is smaller than `right` + // Return value >0: `left` is greater than `right` + // Return value =0: both strings are equal + + int posL = 0; + int posR = 0; + 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 + + const QChar leftChar = (m_caseSensitivity == Qt::CaseSensitive) ? left[posL] : left[posL].toLower(); + const QChar rightChar = (m_caseSensitivity == Qt::CaseSensitive) ? right[posR] : right[posR].toLower(); + if (leftChar == rightChar) { + // compare next character + ++posL; + ++posR; + } + else if (leftChar.isDigit() && rightChar.isDigit()) { + // Both are digits, compare the numbers + const auto consumeNumber = [](const QString &str, int &pos) -> int + { + const int start = pos; + while ((pos < str.size()) && str[pos].isDigit()) + ++pos; + return str.midRef(start, (pos - start)).toInt(); + }; + + const int numL = consumeNumber(left, posL); + const int numR = consumeNumber(right, posR); + if (numL != numR) + return (numL - numR); + + // String + digits do match and we haven't hit the end of both strings + // then continue to consume the remainings + } + else { + return (leftChar.unicode() - rightChar.unicode()); + } + } + } + QCollator m_collator; - const bool m_caseSensitive; + const Qt::CaseSensitivity m_caseSensitivity; }; } -bool Utils::String::naturalCompareCaseSensitive(const QString &left, const QString &right) +int Utils::String::naturalCompare(const QString &left, const QString &right, const Qt::CaseSensitivity caseSensitivity) { // provide a single `NaturalCompare` instance for easy use // https://doc.qt.io/qt-5/threads-reentrancy.html + if (caseSensitivity == Qt::CaseSensitive) { #ifdef Q_OS_MAC // workaround for Apple xcode: https://stackoverflow.com/a/29929949 - static QThreadStorage nCmp; - if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(true)); - return (nCmp.localData())(left, right); + static QThreadStorage nCmp; + if (!nCmp.hasLocalData()) + nCmp.setLocalData(NaturalCompare(Qt::CaseSensitive)); + return (nCmp.localData())(left, right); #else - thread_local NaturalCompare nCmp(true); - return nCmp(left, right); + thread_local NaturalCompare nCmp(Qt::CaseSensitive); + return nCmp(left, right); #endif -} + } -bool Utils::String::naturalCompareCaseInsensitive(const QString &left, const QString &right) -{ - // 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 +#ifdef Q_OS_MAC static QThreadStorage nCmp; - if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(false)); + if (!nCmp.hasLocalData()) + nCmp.setLocalData(NaturalCompare(Qt::CaseInsensitive)); return (nCmp.localData())(left, right); #else - thread_local NaturalCompare nCmp(false); + thread_local NaturalCompare nCmp(Qt::CaseInsensitive); return nCmp(left, right); #endif } @@ -188,4 +181,3 @@ QString Utils::String::wildcardToRegex(const QString &pattern) { return qt_regexp_toCanonical(pattern, QRegExp::Wildcard); } - diff --git a/src/base/utils/string.h b/src/base/utils/string.h index e5257de14..5a8041955 100644 --- a/src/base/utils/string.h +++ b/src/base/utils/string.h @@ -45,8 +45,12 @@ namespace Utils // Taken from https://crackstation.net/hashing-security.htm bool slowEquals(const QByteArray &a, const QByteArray &b); - bool naturalCompareCaseSensitive(const QString &left, const QString &right); - bool naturalCompareCaseInsensitive(const QString &left, const QString &right); + int naturalCompare(const QString &left, const QString &right, const Qt::CaseSensitivity caseSensitivity); + template + bool naturalLessThan(const QString &left, const QString &right) + { + return (naturalCompare(left, right, caseSensitivity) < 0); + } QString wildcardToRegex(const QString &pattern); diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 88fc896a8..39f430363 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -128,7 +128,7 @@ AddNewTorrentDialog::AddNewTorrentDialog(const BitTorrent::AddTorrentParams &inP // Load categories QStringList categories = session->categories().keys(); - std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive); + std::sort(categories.begin(), categories.end(), Utils::String::naturalLessThan); QString defaultCategory = settings()->loadValue(KEY_DEFAULTCATEGORY).toString(); if (!m_torrentParams.category.isEmpty()) diff --git a/src/gui/categoryfilterproxymodel.cpp b/src/gui/categoryfilterproxymodel.cpp index 4862aa238..f7066d97e 100644 --- a/src/gui/categoryfilterproxymodel.cpp +++ b/src/gui/categoryfilterproxymodel.cpp @@ -50,8 +50,12 @@ bool CategoryFilterProxyModel::lessThan(const QModelIndex &left, const QModelInd { // "All" and "Uncategorized" must be left in place if (CategoryFilterModel::isSpecialItem(left) || CategoryFilterModel::isSpecialItem(right)) - return left.row() < right.row(); - else - return Utils::String::naturalCompareCaseInsensitive( - left.data().toString(), right.data().toString()); + return (left < right); + + int result = Utils::String::naturalCompare(left.data().toString(), right.data().toString() + , Qt::CaseInsensitive); + if (result != 0) + return (result < 0); + + return (mapFromSource(left) < mapFromSource(right)); } diff --git a/src/gui/properties/peerlistsortmodel.h b/src/gui/properties/peerlistsortmodel.h index 77e58f377..c1a78c224 100644 --- a/src/gui/properties/peerlistsortmodel.h +++ b/src/gui/properties/peerlistsortmodel.h @@ -50,13 +50,21 @@ protected: switch (sortColumn()) { case PeerListDelegate::IP: case PeerListDelegate::CLIENT: { - QString vL = left.data().toString(); - QString vR = right.data().toString(); - return Utils::String::naturalCompareCaseInsensitive(vL, vR); - } - }; + const QString strL = left.data().toString(); + const QString strR = right.data().toString(); + const int result = Utils::String::naturalCompare(strL, strR, Qt::CaseInsensitive); + if (result != 0) + return (result < 0); - return QSortFilterProxyModel::lessThan(left, right); + return (mapFromSource(left) < mapFromSource(right)); + } + break; + default: + if (left.data() != right.data()) + return QSortFilterProxyModel::lessThan(left, right); + + return (mapFromSource(left) < mapFromSource(right)); + }; } }; diff --git a/src/gui/rss/automatedrssdownloader.cpp b/src/gui/rss/automatedrssdownloader.cpp index dc2cf0aa6..71a7ce311 100644 --- a/src/gui/rss/automatedrssdownloader.cpp +++ b/src/gui/rss/automatedrssdownloader.cpp @@ -311,7 +311,7 @@ void AutomatedRssDownloader::initCategoryCombobox() { // Load torrent categories QStringList categories = BitTorrent::Session::instance()->categories().keys(); - std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive); + std::sort(categories.begin(), categories.end(), Utils::String::naturalLessThan); m_ui->comboCategory->addItem(""); m_ui->comboCategory->addItems(categories); } diff --git a/src/gui/search/searchsortmodel.cpp b/src/gui/search/searchsortmodel.cpp index f5d24ff45..7a531ee7b 100644 --- a/src/gui/search/searchsortmodel.cpp +++ b/src/gui/search/searchsortmodel.cpp @@ -110,13 +110,20 @@ bool SearchSortModel::lessThan(const QModelIndex &left, const QModelIndex &right switch (sortColumn()) { case NAME: case ENGINE_URL: { - QString vL = left.data().toString(); - QString vR = right.data().toString(); - return Utils::String::naturalCompareCaseInsensitive(vL, vR); - } + const QString strL = left.data().toString(); + const QString strR = right.data().toString(); + const int result = Utils::String::naturalCompare(strL, strR, Qt::CaseInsensitive); + if (result != 0) + return (result < 0); + return (mapFromSource(left) < mapFromSource(right)); + } + break; default: - return base::lessThan(left, right); + if (left.data() != right.data()) + return base::lessThan(left, right); + + return (mapFromSource(left) < mapFromSource(right)); }; } diff --git a/src/gui/tagfilterproxymodel.cpp b/src/gui/tagfilterproxymodel.cpp index 5bfb9036f..ffe572e8e 100644 --- a/src/gui/tagfilterproxymodel.cpp +++ b/src/gui/tagfilterproxymodel.cpp @@ -50,7 +50,12 @@ bool TagFilterProxyModel::lessThan(const QModelIndex &left, const QModelIndex &r { // "All" and "Untagged" must be left in place if (TagFilterModel::isSpecialItem(left) || TagFilterModel::isSpecialItem(right)) - return left.row() < right.row(); - return Utils::String::naturalCompareCaseInsensitive( - left.data().toString(), right.data().toString()); + return (left < right); + + int result = Utils::String::naturalCompare(left.data().toString(), right.data().toString() + , Qt::CaseInsensitive); + if (result != 0) + return (result < 0); + + return (mapFromSource(left) < mapFromSource(right)); } diff --git a/src/gui/torrentcontentfiltermodel.cpp b/src/gui/torrentcontentfiltermodel.cpp index c765f2bd6..4bb665804 100644 --- a/src/gui/torrentcontentfiltermodel.cpp +++ b/src/gui/torrentcontentfiltermodel.cpp @@ -88,21 +88,24 @@ bool TorrentContentFilterModel::lessThan(const QModelIndex &left, const QModelIn { switch (sortColumn()) { case TorrentContentModelItem::COL_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())); + const TorrentContentModelItem::ItemType leftType = m_model->itemType(m_model->index(left.row(), 0, left.parent())); + const TorrentContentModelItem::ItemType rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent())); - if (leftType == rightType) - return Utils::String::naturalCompareCaseInsensitive(vL, vR); - else if ((leftType == TorrentContentModelItem::FolderType) && (sortOrder() == Qt::AscendingOrder)) + if (leftType == rightType) { + const QString strL = left.data().toString(); + const QString strR = right.data().toString(); + return Utils::String::naturalLessThan(strL, strR); + } + else if ((leftType == TorrentContentModelItem::FolderType) && (sortOrder() == Qt::AscendingOrder)) { return true; - else + } + else { return false; + } } + default: + return QSortFilterProxyModel::lessThan(left, right); }; - - return QSortFilterProxyModel::lessThan(left, right); } void TorrentContentFilterModel::selectAll() diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index 6f750ad97..9f9b1a550 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -259,7 +259,7 @@ void TrackerFiltersList::addItem(const QString &tracker, const QString &hash) Q_ASSERT(count() >= 4); int insPos = count(); for (int i = 4; i < count(); ++i) { - if (Utils::String::naturalCompareCaseSensitive(host, item(i)->text())) { + if (Utils::String::naturalLessThan(host, item(i)->text())) { insPos = i; break; } diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index 2bf3051e9..97d80afd5 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -89,12 +89,16 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex case TorrentModel::TR_CATEGORY: case TorrentModel::TR_TAGS: case TorrentModel::TR_NAME: { - QVariant vL = left.data(); - QVariant vR = right.data(); + const QVariant vL = left.data(); + const QVariant vR = right.data(); if (!vL.isValid() || !vR.isValid() || (vL == vR)) return lowerPositionThan(left, right); - return Utils::String::naturalCompareCaseInsensitive(vL.toString(), vR.toString()); + const int result = Utils::String::naturalCompare(vL.toString(), vR.toString(), Qt::CaseInsensitive); + if (result != 0) + return (result < 0); + + return (mapFromSource(left) < mapFromSource(right)); } case TorrentModel::TR_ADD_DATE: @@ -109,59 +113,61 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex 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(); - int right_total = right.data(Qt::UserRole).toInt(); + const int leftActive = left.data().toInt(); + const int leftTotal = left.data(Qt::UserRole).toInt(); + const int rightActive = right.data().toInt(); + const int rightTotal = right.data(Qt::UserRole).toInt(); // Active peers/seeds take precedence over total peers/seeds. - if (left_active == right_active) { - if (left_total == right_total) - return lowerPositionThan(left, right); - return (left_total < right_total); - } - else { - return (left_active < right_active); - } + if (leftActive != rightActive) + return (leftActive < rightActive); + + if (leftTotal != rightTotal) + return (leftTotal < rightTotal); + + return lowerPositionThan(left, right); } case TorrentModel::TR_ETA: { - TorrentModel *model = qobject_cast(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(); - const qlonglong etaL = left.data().toLongLong(); - const qlonglong etaR = right.data().toLongLong(); - const bool ascend = (sortOrder() == Qt::AscendingOrder); - const bool invalidL = (etaL < 0 || etaL >= MAX_ETA); - const bool invalidR = (etaR < 0 || etaR >= MAX_ETA); - const bool seedingL = (prioL < 0); - const bool seedingR = (prioR < 0); - - bool activeR = TorrentFilter::ActiveTorrent.match(model->torrentHandle(model->index(right.row()))); - bool activeL = TorrentFilter::ActiveTorrent.match(model->torrentHandle(model->index(left.row()))); + const TorrentModel *model = qobject_cast(sourceModel()); // Sorting rules prioritized. // 1. Active torrents at the top // 2. Seeding torrents at the bottom // 3. Torrents with invalid ETAs at the bottom - if (activeL != activeR) return activeL; - if (seedingL != seedingR) { - if (seedingL) return !ascend; - else return ascend; + const bool isActiveL = TorrentFilter::ActiveTorrent.match(model->torrentHandle(model->index(left.row()))); + const bool isActiveR = TorrentFilter::ActiveTorrent.match(model->torrentHandle(model->index(right.row()))); + if (isActiveL != isActiveR) + return isActiveL; + + 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(); + const bool isSeedingL = (prioL < 0); + const bool isSeedingR = (prioR < 0); + if (isSeedingL != isSeedingR) { + const bool isAscendingOrder = (sortOrder() == Qt::AscendingOrder); + if (isSeedingL) + return !isAscendingOrder; + else + return isAscendingOrder; } - if (invalidL && invalidR) { - if (seedingL) // Both seeding + const qlonglong etaL = left.data().toLongLong(); + const qlonglong etaR = right.data().toLongLong(); + const bool isInvalidL = ((etaL < 0) || (etaL >= MAX_ETA)); + const bool isInvalidR = ((etaR < 0) || (etaR >= MAX_ETA)); + if (isInvalidL && isInvalidR) { + if (isSeedingL) // Both seeding return dateLessThan(TorrentModel::TR_SEED_DATE, left, right, true); else - return prioL < prioR; + return (prioL < prioR); } - else if (!invalidL && !invalidR) { - return etaL < etaR; + else if (!isInvalidL && !isInvalidR) { + return (etaL < etaR); } else { - return !invalidL; + return !isInvalidL; } } @@ -186,9 +192,10 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex } default: { - if (left.data() == right.data()) - return lowerPositionThan(left, right); - return QSortFilterProxyModel::lessThan(left, right); + if (left.data() != right.data()) + return QSortFilterProxyModel::lessThan(left, right); + + return lowerPositionThan(left, right); } } } diff --git a/src/gui/transferlistwidget.cpp b/src/gui/transferlistwidget.cpp index 380ffc636..756588b23 100644 --- a/src/gui/transferlistwidget.cpp +++ b/src/gui/transferlistwidget.cpp @@ -988,7 +988,7 @@ void TransferListWidget::displayListMenu(const QPoint&) listMenu.addAction(&actionRename); // Category Menu QStringList categories = BitTorrent::Session::instance()->categories().keys(); - std::sort(categories.begin(), categories.end(), Utils::String::naturalCompareCaseInsensitive); + std::sort(categories.begin(), categories.end(), Utils::String::naturalLessThan); QList categoryActions; QMenu *categoryMenu = listMenu.addMenu(GuiIconProvider::instance()->getIcon("view-categories"), tr("Category")); categoryActions << categoryMenu->addAction(GuiIconProvider::instance()->getIcon("list-add"), tr("New...", "New category...")); @@ -1007,7 +1007,7 @@ void TransferListWidget::displayListMenu(const QPoint&) // Tag Menu QStringList tags(BitTorrent::Session::instance()->tags().toList()); - std::sort(tags.begin(), tags.end(), Utils::String::naturalCompareCaseInsensitive); + std::sort(tags.begin(), tags.end(), Utils::String::naturalLessThan); QList tagsActions; QMenu *tagsMenu = listMenu.addMenu(GuiIconProvider::instance()->getIcon("view-categories"), tr("Tags")); tagsActions << tagsMenu->addAction(GuiIconProvider::instance()->getIcon("list-add"), tr("Add...", "Add / assign multiple tags..."));