From 5906a4a2dea9df48f0a03bc8cbb03c312322f489 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 4 May 2016 17:15:58 +0800 Subject: [PATCH] Add case-sensitive & case-insensitive natural sort helper function Fix helper function not being thread-safe Use QBT_USES_QT5 define --- src/base/utils/string.cpp | 181 ++++++++++++++----------- src/base/utils/string.h | 3 +- src/gui/addnewtorrentdialog.cpp | 2 +- src/gui/properties/peerlistsortmodel.h | 2 +- src/gui/rss/automatedrssdownloader.cpp | 2 +- src/gui/search/searchsortmodel.cpp | 2 +- src/gui/torrentcontentfiltermodel.cpp | 2 +- src/gui/transferlistfilterswidget.cpp | 4 +- src/gui/transferlistsortmodel.cpp | 2 +- src/gui/transferlistwidget.cpp | 2 +- 10 files changed, 113 insertions(+), 89 deletions(-) diff --git a/src/base/utils/string.cpp b/src/base/utils/string.cpp index ccd35940c..37a00713e 100644 --- a/src/base/utils/string.cpp +++ b/src/base/utils/string.cpp @@ -34,100 +34,130 @@ #include #include #include -#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) +#ifdef QBT_USES_QT5 #include #endif - -class NaturalCompare -{ -public: - NaturalCompare(); - bool operator()(const QString &left, const QString &right); - bool lessThan(const QString &left, const QString &right); - -private: -#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) - QCollator m_collator; +#ifdef Q_OS_MAC +#include #endif -}; -NaturalCompare::NaturalCompare() +namespace { -#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) + 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; + // 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); + m_collator.setNumericMode(true); + m_collator.setCaseSensitivity(caseSensitive ? Qt::CaseSensitive : Qt::CaseInsensitive); #endif -} + } -bool NaturalCompare::operator()(const QString &left, const QString &right) -{ - // case-insensitive comparison -#if (QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)) + 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); + // 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); + return (m_collator.compare(left, right) < 0); #else - return lessThan(left, right); + return lessThan(left, right); #endif -} - -bool NaturalCompare::lessThan(const QString &left, const QString &right) -{ - // Return value `false` indicates `right` should go before `left`, otherwise, after - // case-insensitive comparison - 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(); + 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(); + 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(); + 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(); + int numR = right.mid(startR, posR - startR).toInt(); #endif - if (numL != numR) - return (numL < numR); + if (numL != numR) + return (numL < numR); + + // Strings + digits do match and we haven't hit string end + // Do another round + } + return false; + } + + private: +#ifdef QBT_USES_QT5 + QCollator m_collator; +#endif + const bool m_caseSensitive; + }; +} + +bool Utils::String::naturalCompareCaseSensitive(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 + static QThreadStorage nCmp; + if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(true)); + return (nCmp.localData())(left, right); +#else + thread_local NaturalCompare nCmp(true); + return nCmp(left, right); +#endif +} - // Strings + digits do match and we haven't hit string end - // Do another round - } - return false; +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 + static QThreadStorage nCmp; + if (!nCmp.hasLocalData()) nCmp.setLocalData(NaturalCompare(false)); + return (nCmp.localData())(left, right); +#else + thread_local NaturalCompare nCmp(false); + return nCmp(left, right); +#endif } QString Utils::String::fromStdString(const std::string &str) @@ -141,13 +171,6 @@ std::string Utils::String::toStdString(const QString &str) return std::string(utf8.constData(), utf8.length()); } -bool Utils::String::naturalCompare(const QString &left, const QString &right) -{ - // provide a single `NaturalCompare` instance for easy use - static NaturalCompare nCmp; // this is thread-safe in C++11 (stated in spec 6.7.4) - return nCmp(left, right); -} - // to send numbers instead of strings with suffixes QString Utils::String::fromDouble(double n, int precision) { diff --git a/src/base/utils/string.h b/src/base/utils/string.h index 9a3c66843..8ca2f2e45 100644 --- a/src/base/utils/string.h +++ b/src/base/utils/string.h @@ -47,7 +47,8 @@ namespace Utils // Taken from https://crackstation.net/hashing-security.htm bool slowEquals(const QByteArray &a, const QByteArray &b); - bool naturalCompare(const QString &left, const QString &right); + bool naturalCompareCaseSensitive(const QString &left, const QString &right); + bool naturalCompareCaseInsensitive(const QString &left, const QString &right); } } diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 3511ef73f..a689c0b86 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/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()) diff --git a/src/gui/properties/peerlistsortmodel.h b/src/gui/properties/peerlistsortmodel.h index ac298a3da..729d92bfa 100644 --- a/src/gui/properties/peerlistsortmodel.h +++ b/src/gui/properties/peerlistsortmodel.h @@ -48,7 +48,7 @@ protected: case PeerListDelegate::CLIENT: { QString vL = left.data().toString(); QString vR = right.data().toString(); - return Utils::String::naturalCompare(vL, vR); + return Utils::String::naturalCompareCaseSensitive(vL, vR); } }; diff --git a/src/gui/rss/automatedrssdownloader.cpp b/src/gui/rss/automatedrssdownloader.cpp index 744229710..4396d106e 100644 --- a/src/gui/rss/automatedrssdownloader.cpp +++ b/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); } diff --git a/src/gui/search/searchsortmodel.cpp b/src/gui/search/searchsortmodel.cpp index 0052029eb..e27af9fc8 100644 --- a/src/gui/search/searchsortmodel.cpp +++ b/src/gui/search/searchsortmodel.cpp @@ -112,7 +112,7 @@ bool SearchSortModel::lessThan(const QModelIndex &left, const QModelIndex &right case ENGINE_URL: { QString vL = left.data().toString(); QString vR = right.data().toString(); - return Utils::String::naturalCompare(vL, vR); + return Utils::String::naturalCompareCaseSensitive(vL, vR); } default: diff --git a/src/gui/torrentcontentfiltermodel.cpp b/src/gui/torrentcontentfiltermodel.cpp index fb4d849b1..bb957c92d 100644 --- a/src/gui/torrentcontentfiltermodel.cpp +++ b/src/gui/torrentcontentfiltermodel.cpp @@ -90,7 +90,7 @@ bool TorrentContentFilterModel::lessThan(const QModelIndex &left, const QModelIn TorrentContentModelItem::ItemType rightType = m_model->itemType(m_model->index(right.row(), 0, right.parent())); if (leftType == rightType) - return Utils::String::naturalCompare(vL, vR); + return Utils::String::naturalCompareCaseSensitive(vL, vR); else if (leftType == TorrentContentModelItem::FolderType && sortOrder() == Qt::AscendingOrder) return true; else diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index a2da34141..eaa8e9d01 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -239,7 +239,7 @@ void CategoryFiltersList::addItem(const QString &category, bool hasTorrent) Q_ASSERT(count() >= 2); int insPos = count(); for (int i = 2; i < count(); ++i) { - if (Utils::String::naturalCompare(category, item(i)->text())) { + if (Utils::String::naturalCompareCaseSensitive(category, item(i)->text())) { insPos = i; break; } @@ -511,7 +511,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::naturalCompare(host, item(i)->text())) { + if (Utils::String::naturalCompareCaseSensitive(host, item(i)->text())) { insPos = i; break; } diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index 06eeed220..2058ce91f 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -80,7 +80,7 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex if (!vL.isValid() || !vR.isValid() || (vL == vR)) return lowerPositionThan(left, right); - return Utils::String::naturalCompare(vL.toString(), vR.toString()); + return Utils::String::naturalCompareCaseSensitive(vL.toString(), vR.toString()); } case TorrentModel::TR_ADD_DATE: diff --git a/src/gui/transferlistwidget.cpp b/src/gui/transferlistwidget.cpp index d147f8c6c..c7e25409e 100644 --- a/src/gui/transferlistwidget.cpp +++ b/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 categoryActions; QMenu *categoryMenu = listMenu.addMenu(GuiIconProvider::instance()->getIcon("view-categories"), tr("Category")); categoryActions << categoryMenu->addAction(GuiIconProvider::instance()->getIcon("list-add"), tr("New...", "New category..."));