Fix transfer list architecture

Model returns string for DisplayRole.
Text alignment is set by Model (using TextAlignmentRole).
Delegate performs custom painting only where necessary
(i.e. for Progress bar).

src/gui/transferlistdelegate.cpp

@@ -29,7 +29,6 @@
 #include "transferlistdelegate.h"
 
 #include <QApplication>
-#include <QDateTime>
 #include <QModelIndex>
 #include <QPainter>
 #include <QStyleOptionViewItem>
@@ -38,11 +37,6 @@
 #include <QProxyStyle>
 #endif
 
-#include "base/bittorrent/torrenthandle.h"
-#include "base/preferences.h"
-#include "base/types.h"
-#include "base/unicodestrings.h"
-#include "base/utils/misc.h"
 #include "base/utils/string.h"
 #include "transferlistmodel.h"
 
@@ -53,168 +47,32 @@ TransferListDelegate::TransferListDelegate(QObject *parent)
 
 void TransferListDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const
 {
-    painter->save();
-
-    bool isHideState = true;
-    if (Preferences::instance()->getHideZeroComboValues() == 1) {  // paused torrents only
-        const QModelIndex stateIndex = index.sibling(index.row(), TransferListModel::TR_STATUS);
-        if (stateIndex.data().value<BitTorrent::TorrentState>() != BitTorrent::TorrentState::PausedDownloading)
-            isHideState = false;
+    if (index.column() != TransferListModel::TR_PROGRESS) {
+        QItemDelegate::paint(painter, option, index);
+        return;
     }
-    const bool hideValues = Preferences::instance()->getHideZeroValues() && isHideState;
+
+    painter->save();
 
     QStyleOptionViewItem opt = QItemDelegate::setOptions(index, option);
     QItemDelegate::drawBackground(painter, opt, index);
 
-    switch (index.column()) {
-    case TransferListModel::TR_AMOUNT_DOWNLOADED:
-    case TransferListModel::TR_AMOUNT_UPLOADED:
-    case TransferListModel::TR_AMOUNT_DOWNLOADED_SESSION:
-    case TransferListModel::TR_AMOUNT_UPLOADED_SESSION:
-    case TransferListModel::TR_AMOUNT_LEFT:
-    case TransferListModel::TR_COMPLETED:
-    case TransferListModel::TR_SIZE:
-    case TransferListModel::TR_TOTAL_SIZE: {
-            qlonglong size = index.data().toLongLong();
-            if (hideValues && !size)
-                break;
-            opt.displayAlignment = (Qt::AlignRight | Qt::AlignVCenter);
-            QItemDelegate::drawDisplay(painter, opt, option.rect, Utils::Misc::friendlyUnit(size));
-        }
-        break;
-    case TransferListModel::TR_ETA: {
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, option.rect, Utils::Misc::userFriendlyDuration(index.data().toLongLong(), MAX_ETA));
-        }
-        break;
-    case TransferListModel::TR_SEEDS:
-    case TransferListModel::TR_PEERS: {
-            qlonglong value = index.data().toLongLong();
-            qlonglong total = index.data(Qt::UserRole).toLongLong();
-            if (hideValues && (!value && !total))
-                break;
-            QString display = QString::number(value) + " (" + QString::number(total) + ')';
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, display);
-        }
-        break;
-    case TransferListModel::TR_STATUS: {
-            const auto state = index.data().value<BitTorrent::TorrentState>();
-            const QString errorMsg = index.data(Qt::UserRole).toString();
-            QString display = getStatusString(state);
-            if (state == BitTorrent::TorrentState::Error)
-                display += (": " + errorMsg);
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, display);
-        }
-        break;
-    case TransferListModel::TR_UPSPEED:
-    case TransferListModel::TR_DLSPEED: {
-            const int speed = index.data().toInt();
-            if (hideValues && !speed)
-                break;
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, Utils::Misc::friendlyUnit(speed, true));
-        }
-        break;
-    case TransferListModel::TR_UPLIMIT:
-    case TransferListModel::TR_DLLIMIT: {
-            const qlonglong limit = index.data().toLongLong();
-            if (hideValues && !limit)
-                break;
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, limit > 0 ? Utils::Misc::friendlyUnit(limit, true) : QString::fromUtf8(C_INFINITY));
-        }
-        break;
-    case TransferListModel::TR_TIME_ELAPSED: {
-            const qlonglong elapsedTime = index.data().toLongLong();
-            const qlonglong seedingTime = index.data(Qt::UserRole).toLongLong();
-            const QString txt = (seedingTime > 0)
-                ? tr("%1 (seeded for %2)", "e.g. 4m39s (seeded for 3m10s)")
-                    .arg(Utils::Misc::userFriendlyDuration(elapsedTime)
-                        , Utils::Misc::userFriendlyDuration(seedingTime))
-                : Utils::Misc::userFriendlyDuration(elapsedTime);
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, txt);
-        }
-        break;
-    case TransferListModel::TR_ADD_DATE:
-    case TransferListModel::TR_SEED_DATE:
-        QItemDelegate::drawDisplay(painter, opt, opt.rect, index.data().toDateTime().toLocalTime().toString(Qt::DefaultLocaleShortDate));
-        break;
-    case TransferListModel::TR_RATIO_LIMIT:
-    case TransferListModel::TR_RATIO: {
-            const qreal ratio = index.data().toDouble();
-            if (hideValues && (ratio <= 0))
-                break;
-            QString str = ((ratio == -1) || (ratio > BitTorrent::TorrentHandle::MAX_RATIO)) ? QString::fromUtf8(C_INFINITY) : Utils::String::fromDouble(ratio, 2);
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, opt.rect, str);
-        }
-        break;
-    case TransferListModel::TR_QUEUE_POSITION: {
-            const int queuePos = index.data().toInt();
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            if (queuePos > 0)
-                QItemDelegate::paint(painter, opt, index);
-            else
-                QItemDelegate::drawDisplay(painter, opt, opt.rect, "*");
-        }
-        break;
-    case TransferListModel::TR_PROGRESS: {
-            const qreal progress = index.data().toDouble() * 100;
-
-            QStyleOptionProgressBar newopt;
-            newopt.rect = opt.rect;
-            newopt.text = ((progress == 100)
-                ? QString("100%")
-                : (Utils::String::fromDouble(progress, 1) + '%'));
-            newopt.progress = static_cast<int>(progress);
-            newopt.maximum = 100;
-            newopt.minimum = 0;
-            newopt.state |= QStyle::State_Enabled;
-            newopt.textVisible = true;
+    QStyleOptionProgressBar newopt;
+    newopt.rect = opt.rect;
+    newopt.text = index.data().toString();
+    newopt.progress = static_cast<int>(index.data(TransferListModel::UnderlyingDataRole).toReal());
+    newopt.maximum = 100;
+    newopt.minimum = 0;
+    newopt.state |= QStyle::State_Enabled;
+    newopt.textVisible = true;
 
 #if defined(Q_OS_WIN) || defined(Q_OS_MACOS)
-           // XXX: To avoid having the progress text on the right of the bar
-            QProxyStyle st("fusion");
-            st.drawControl(QStyle::CE_ProgressBar, &newopt, painter);
+    // XXX: To avoid having the progress text on the right of the bar
+    QProxyStyle st("fusion");
+    st.drawControl(QStyle::CE_ProgressBar, &newopt, painter);
 #else
-            QApplication::style()->drawControl(QStyle::CE_ProgressBar, &newopt, painter);
+    QApplication::style()->drawControl(QStyle::CE_ProgressBar, &newopt, painter);
 #endif
-        }
-        break;
-    case TransferListModel::TR_LAST_ACTIVITY: {
-            qlonglong elapsed = index.data().toLongLong();
-            if (hideValues && ((elapsed < 0) || (elapsed >= MAX_ETA)))
-                break;
-
-            // Show '< 1m ago' when elapsed time is 0
-            if (elapsed == 0)
-                elapsed = 1;
-
-            QString elapsedString = (elapsed >= 0)
-                ? tr("%1 ago", "e.g.: 1h 20m ago").arg(Utils::Misc::userFriendlyDuration(elapsed))
-                : Utils::Misc::userFriendlyDuration(elapsed);
-
-            opt.displayAlignment = Qt::AlignRight | Qt::AlignVCenter;
-            QItemDelegate::drawDisplay(painter, opt, option.rect, elapsedString);
-        }
-        break;
-
-    case TransferListModel::TR_AVAILABILITY: {
-            const qreal availability = index.data().toReal();
-            if (hideValues && (availability <= 0))
-                break;
-
-            const QString availabilityStr = Utils::String::fromDouble(availability, 3);
-            opt.displayAlignment = (Qt::AlignRight | Qt::AlignVCenter);
-            QItemDelegate::drawDisplay(painter, opt, option.rect, availabilityStr);
-        }
-        break;
-
-    default:
-        QItemDelegate::paint(painter, option, index);
-    }
-
     painter->restore();
 }
 
@@ -241,44 +99,3 @@ QSize TransferListDelegate::sizeHint(const QStyleOptionViewItem &option, const Q
     size.setHeight(std::max(nameColHeight, size.height()));
     return size;
 }
-
-QString TransferListDelegate::getStatusString(const BitTorrent::TorrentState state) const
-{
-    switch (state) {
-    case BitTorrent::TorrentState::Downloading:
-        return tr("Downloading");
-    case BitTorrent::TorrentState::StalledDownloading:
-        return tr("Stalled", "Torrent is waiting for download to begin");
-    case BitTorrent::TorrentState::DownloadingMetadata:
-        return tr("Downloading metadata", "Used when loading a magnet link");
-    case BitTorrent::TorrentState::ForcedDownloading:
-        return tr("[F] Downloading", "Used when the torrent is forced started. You probably shouldn't translate the F.");
-    case BitTorrent::TorrentState::Allocating:
-        return tr("Allocating", "qBittorrent is allocating the files on disk");
-    case BitTorrent::TorrentState::Uploading:
-    case BitTorrent::TorrentState::StalledUploading:
-        return tr("Seeding", "Torrent is complete and in upload-only mode");
-    case BitTorrent::TorrentState::ForcedUploading:
-        return tr("[F] Seeding", "Used when the torrent is forced started. You probably shouldn't translate the F.");
-    case BitTorrent::TorrentState::QueuedDownloading:
-    case BitTorrent::TorrentState::QueuedUploading:
-        return tr("Queued", "Torrent is queued");
-    case BitTorrent::TorrentState::CheckingDownloading:
-    case BitTorrent::TorrentState::CheckingUploading:
-        return tr("Checking", "Torrent local data is being checked");
-    case BitTorrent::TorrentState::CheckingResumeData:
-        return tr("Checking resume data", "Used when loading the torrents from disk after qbt is launched. It checks the correctness of the .fastresume file. Normally it is completed in a fraction of a second, unless loading many many torrents.");
-    case BitTorrent::TorrentState::PausedDownloading:
-        return tr("Paused");
-    case BitTorrent::TorrentState::PausedUploading:
-        return tr("Completed");
-    case BitTorrent::TorrentState::Moving:
-        return tr("Moving", "Torrent local data are being moved/relocated");
-    case BitTorrent::TorrentState::MissingFiles:
-        return tr("Missing Files");
-    case BitTorrent::TorrentState::Error:
-        return tr("Errored", "Torrent status, the torrent has an error");
-    default:
-        return {};
-    }
-}

src/gui/transferlistdelegate.h

@@ -35,11 +35,6 @@ class QModelIndex;
 class QPainter;
 class QStyleOptionViewItem;
 
-namespace BitTorrent
-{
-    enum class TorrentState;
-}
-
 class TransferListDelegate : public QItemDelegate
 {
     Q_OBJECT
@@ -49,9 +44,6 @@ public:
     void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
     QWidget *createEditor(QWidget *, const QStyleOptionViewItem &, const QModelIndex &) const override;
     QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override;
-
-private:
-    QString getStatusString(const BitTorrent::TorrentState state) const;
 };
 
 #endif // TRANSFERLISTDELEGATE_H

src/gui/transferlistmodel.cpp

@@ -38,7 +38,11 @@
 #include "base/bittorrent/session.h"
 #include "base/bittorrent/torrenthandle.h"
 #include "base/global.h"
+#include "base/preferences.h"
+#include "base/unicodestrings.h"
 #include "base/utils/fs.h"
+#include "base/utils/misc.h"
+#include "base/utils/string.h"
 
 static QIcon getIconByState(BitTorrent::TorrentState state);
 static QColor getColorByState(BitTorrent::TorrentState state);
@@ -58,7 +62,27 @@ static bool isDarkTheme();
 // TransferListModel
 
 TransferListModel::TransferListModel(QObject *parent)
-    : QAbstractListModel(parent)
+    : QAbstractListModel {parent}
+    , m_statusStrings {
+          {BitTorrent::TorrentState::Downloading, tr("Downloading")},
+          {BitTorrent::TorrentState::StalledDownloading, tr("Stalled", "Torrent is waiting for download to begin")},
+          {BitTorrent::TorrentState::DownloadingMetadata, tr("Downloading metadata", "Used when loading a magnet link")},
+          {BitTorrent::TorrentState::ForcedDownloading, tr("[F] Downloading", "Used when the torrent is forced started. You probably shouldn't translate the F.")},
+          {BitTorrent::TorrentState::Allocating, tr("Allocating", "qBittorrent is allocating the files on disk")},
+          {BitTorrent::TorrentState::Uploading, tr("Seeding", "Torrent is complete and in upload-only mode")},
+          {BitTorrent::TorrentState::StalledUploading, tr("Seeding", "Torrent is complete and in upload-only mode")},
+          {BitTorrent::TorrentState::ForcedUploading, tr("[F] Seeding", "Used when the torrent is forced started. You probably shouldn't translate the F.")},
+          {BitTorrent::TorrentState::QueuedDownloading, tr("Queued", "Torrent is queued")},
+          {BitTorrent::TorrentState::QueuedUploading, tr("Queued", "Torrent is queued")},
+          {BitTorrent::TorrentState::CheckingDownloading, tr("Checking", "Torrent local data is being checked")},
+          {BitTorrent::TorrentState::CheckingUploading, tr("Checking", "Torrent local data is being checked")},
+          {BitTorrent::TorrentState::CheckingResumeData, tr("Checking resume data", "Used when loading the torrents from disk after qbt is launched. It checks the correctness of the .fastresume file. Normally it is completed in a fraction of a second, unless loading many many torrents.")},
+          {BitTorrent::TorrentState::PausedDownloading, tr("Paused")},
+          {BitTorrent::TorrentState::PausedUploading, tr("Completed")},
+          {BitTorrent::TorrentState::Moving, tr("Moving", "Torrent local data are being moved/relocated")},
+          {BitTorrent::TorrentState::MissingFiles, tr("Missing Files")},
+          {BitTorrent::TorrentState::Error, tr("Errored", "Torrent status, the torrent has an error")}
+      }
     , m_stateForegroundColors {
         {BitTorrent::TorrentState::Unknown, getColorByState(BitTorrent::TorrentState::Unknown)},
         {BitTorrent::TorrentState::ForcedDownloading, getColorByState(BitTorrent::TorrentState::ForcedDownloading)},
@@ -179,23 +203,172 @@ QVariant TransferListModel::headerData(int section, Qt::Orientation orientation,
     return {};
 }
 
-QVariant TransferListModel::data(const QModelIndex &index, const int role) const
+QString TransferListModel::displayValue(const BitTorrent::TorrentHandle *torrent, const int column) const
 {
-    if (!index.isValid()) return {};
-
-    const BitTorrent::TorrentHandle *torrent = m_torrentList.value(index.row());
-    if (!torrent) return {};
-
-    if ((role == Qt::DecorationRole) && (index.column() == TR_NAME))
-        return getIconByState(torrent->state());
-
-    if (role == Qt::ForegroundRole)
-        return stateForeground(torrent->state());
+    const bool isHideState = (Preferences::instance()->getHideZeroComboValues() == 1)
+            && (torrent->state() == BitTorrent::TorrentState::PausedDownloading); // paused torrents only
+    const bool hideValues = Preferences::instance()->getHideZeroValues() && isHideState;
+
+    const auto availabilityString = [hideValues](const qreal value) -> QString
+    {
+        return ((value <= 0) && hideValues)
+                ? QString {} : Utils::String::fromDouble(value, 3);
+    };
+
+    const auto unitString = [hideValues](const qint64 value, const bool isSpeedUnit = false) -> QString
+    {
+        return ((value == 0) && hideValues)
+                ? QString {} : Utils::Misc::friendlyUnit(value, isSpeedUnit);
+    };
+
+    const auto limitString = [hideValues](const qint64 value) -> QString
+    {
+        if ((value == 0) && hideValues)
+            return {};
+
+        return (value > 0)
+                ? Utils::Misc::friendlyUnit(value, true)
+                : QString::fromUtf8(C_INFINITY);
+    };
+
+    const auto amountString = [hideValues](const qint64 value, const qint64 total) -> QString
+    {
+        return ((value == 0) && (total == 0) && hideValues)
+                ? QString {}
+                : QString::number(value) + " (" + QString::number(total) + ')';
+    };
+
+    const auto ratioString = [hideValues](const qreal value) -> QString
+    {
+        if ((value <= 0) && hideValues)
+            return {};
+
+         return ((static_cast<int>(value) == -1) || (value > BitTorrent::TorrentHandle::MAX_RATIO))
+                  ? QString::fromUtf8(C_INFINITY) : Utils::String::fromDouble(value, 2);
+    };
+
+    const auto queuePositionString = [](const qint64 value) -> QString
+    {
+        return (value > 0) ? QString::number(value) : "*";
+    };
+
+    const auto lastActivityString = [hideValues](qint64 value) -> QString
+    {
+        if (hideValues && ((value < 0) || (value >= MAX_ETA)))
+            return QString {};
+
+        // Show '< 1m ago' when elapsed time is 0
+        if (value == 0)
+            value = 1;
+
+        return (value >= 0)
+            ? tr("%1 ago", "e.g.: 1h 20m ago").arg(Utils::Misc::userFriendlyDuration(value))
+            : Utils::Misc::userFriendlyDuration(value);
+    };
+
+    const auto timeElapsedString = [](const qint64 elapsedTime, const qint64 seedingTime) -> QString
+    {
+        if (seedingTime <= 0)
+            return Utils::Misc::userFriendlyDuration(elapsedTime);
+
+        return tr("%1 (seeded for %2)", "e.g. 4m39s (seeded for 3m10s)")
+                .arg(Utils::Misc::userFriendlyDuration(elapsedTime)
+                     , Utils::Misc::userFriendlyDuration(seedingTime));
+    };
+
+    const auto tagsString = [](const QSet<QString> &tags) -> QString
+    {
+        QStringList tagsList = tags.values();
+        tagsList.sort();
+        return tagsList.join(", ");
+    };
+
+    const auto progressString = [](qreal progress) -> QString
+    {
+        progress *= 100;
+        return (static_cast<int>(progress) == 100)
+                ? QString {QLatin1String {"100%"}}
+                : Utils::String::fromDouble(progress, 1) + '%';
+    };
+
+    const auto statusString = [this](const BitTorrent::TorrentState state, const QString &errorMessage) -> QString
+    {
+        return (state == BitTorrent::TorrentState::Error)
+                   ? m_statusStrings[state] + ": " + errorMessage
+                   : m_statusStrings[state];
+    };
+
+    switch (column) {
+    case TR_NAME:
+        return torrent->name();
+    case TR_QUEUE_POSITION:
+        return queuePositionString(torrent->queuePosition());
+    case TR_SIZE:
+        return unitString(torrent->wantedSize());
+    case TR_PROGRESS:
+        return progressString(torrent->progress());
+    case TR_STATUS:
+        return statusString(torrent->state(), torrent->error());
+    case TR_SEEDS:
+        return amountString(torrent->seedsCount(), torrent->totalSeedsCount());
+    case TR_PEERS:
+        return amountString(torrent->leechsCount(), torrent->totalLeechersCount());
+    case TR_DLSPEED:
+        return unitString(torrent->downloadPayloadRate(), true);
+    case TR_UPSPEED:
+        return unitString(torrent->uploadPayloadRate(), true);
+    case TR_ETA:
+        return Utils::Misc::userFriendlyDuration(torrent->eta(), MAX_ETA);
+    case TR_RATIO:
+        return ratioString(torrent->realRatio());
+    case TR_RATIO_LIMIT:
+        return ratioString(torrent->maxRatio());
+    case TR_CATEGORY:
+        return torrent->category();
+    case TR_TAGS:
+        return tagsString(torrent->tags());
+    case TR_ADD_DATE:
+        return torrent->addedTime().toLocalTime().toString(Qt::DefaultLocaleShortDate);
+    case TR_SEED_DATE:
+        return torrent->completedTime().toLocalTime().toString(Qt::DefaultLocaleShortDate);
+    case TR_TRACKER:
+        return torrent->currentTracker();
+    case TR_DLLIMIT:
+        return limitString(torrent->downloadLimit());
+    case TR_UPLIMIT:
+        return limitString(torrent->uploadLimit());
+    case TR_AMOUNT_DOWNLOADED:
+        return unitString(torrent->totalDownload());
+    case TR_AMOUNT_UPLOADED:
+        return unitString(torrent->totalUpload());
+    case TR_AMOUNT_DOWNLOADED_SESSION:
+        return unitString(torrent->totalPayloadDownload());
+    case TR_AMOUNT_UPLOADED_SESSION:
+        return unitString(torrent->totalPayloadUpload());
+    case TR_AMOUNT_LEFT:
+        return unitString(torrent->incompletedSize());
+    case TR_TIME_ELAPSED:
+        return timeElapsedString(torrent->activeTime(), torrent->seedingTime());
+    case TR_SAVE_PATH:
+        return Utils::Fs::toNativePath(torrent->savePath());
+    case TR_COMPLETED:
+        return unitString(torrent->completedSize());
+    case TR_SEEN_COMPLETE_DATE:
+        return torrent->lastSeenComplete().toString();
+    case TR_LAST_ACTIVITY:
+        return lastActivityString((torrent->isPaused() || torrent->isChecking()) ? -1 : torrent->timeSinceActivity());
+    case TR_AVAILABILITY:
+        return availabilityString(torrent->distributedCopies());
+    case TR_TOTAL_SIZE:
+        return unitString(torrent->totalSize());
+    }
 
-    if ((role != Qt::DisplayRole) && (role != Qt::UserRole))
-        return {};
+    return {};
+}
 
-    switch (index.column()) {
+QVariant TransferListModel::internalValue(const BitTorrent::TorrentHandle *torrent, const int column, const bool alt) const
+{
+    switch (column) {
     case TR_NAME:
         return torrent->name();
     case TR_QUEUE_POSITION:
@@ -203,13 +376,13 @@ QVariant TransferListModel::data(const QModelIndex &index, const int role) const
     case TR_SIZE:
         return torrent->wantedSize();
     case TR_PROGRESS:
-        return torrent->progress();
+        return torrent->progress() * 100;
     case TR_STATUS:
-        return (role == Qt::DisplayRole) ? QVariant::fromValue(torrent->state()) : torrent->error();
+        return QVariant::fromValue(torrent->state());
     case TR_SEEDS:
-        return (role == Qt::DisplayRole) ? torrent->seedsCount() : torrent->totalSeedsCount();
+        return !alt ? torrent->seedsCount() : torrent->totalSeedsCount();
     case TR_PEERS:
-        return (role == Qt::DisplayRole) ? torrent->leechsCount() : torrent->totalLeechersCount();
+        return !alt ? torrent->leechsCount() : torrent->totalLeechersCount();
     case TR_DLSPEED:
         return torrent->downloadPayloadRate();
     case TR_UPSPEED:
@@ -220,11 +393,8 @@ QVariant TransferListModel::data(const QModelIndex &index, const int role) const
         return torrent->realRatio();
     case TR_CATEGORY:
         return torrent->category();
-    case TR_TAGS: {
-            QStringList tagsList = torrent->tags().values();
-            tagsList.sort();
-            return tagsList.join(", ");
-        }
+    case TR_TAGS:
+        return QStringList {torrent->tags().values()};
     case TR_ADD_DATE:
         return torrent->addedTime();
     case TR_SEED_DATE:
@@ -246,7 +416,7 @@ QVariant TransferListModel::data(const QModelIndex &index, const int role) const
     case TR_AMOUNT_LEFT:
         return torrent->incompletedSize();
     case TR_TIME_ELAPSED:
-        return (role == Qt::DisplayRole) ? torrent->activeTime() : torrent->seedingTime();
+        return !alt ? torrent->activeTime() : torrent->seedingTime();
     case TR_SAVE_PATH:
         return Utils::Fs::toNativePath(torrent->savePath());
     case TR_COMPLETED:
@@ -256,9 +426,7 @@ QVariant TransferListModel::data(const QModelIndex &index, const int role) const
     case TR_SEEN_COMPLETE_DATE:
         return torrent->lastSeenComplete();
     case TR_LAST_ACTIVITY:
-        if (torrent->isPaused() || torrent->isChecking())
-            return -1;
-        return torrent->timeSinceActivity();
+        return (torrent->isPaused() || torrent->isChecking()) ? -1 : torrent->timeSinceActivity();
     case TR_AVAILABILITY:
         return torrent->distributedCopies();
     case TR_TOTAL_SIZE:
@@ -268,6 +436,55 @@ QVariant TransferListModel::data(const QModelIndex &index, const int role) const
     return {};
 }
 
+QVariant TransferListModel::data(const QModelIndex &index, const int role) const
+{
+    if (!index.isValid()) return {};
+
+    const BitTorrent::TorrentHandle *torrent = m_torrentList.value(index.row());
+    if (!torrent) return {};
+
+    switch (role) {
+    case Qt::ForegroundRole:
+        return stateForeground(torrent->state());
+    case Qt::DisplayRole:
+        return displayValue(torrent, index.column());
+    case UnderlyingDataRole:
+        return internalValue(torrent, index.column());
+    case AdditionalUnderlyingDataRole:
+        return internalValue(torrent, index.column(), true);
+    case Qt::DecorationRole:
+        if (index.column() == TR_NAME)
+            return getIconByState(torrent->state());
+        break;
+    case Qt::TextAlignmentRole:
+        switch (index.column()) {
+        case TR_AMOUNT_DOWNLOADED:
+        case TR_AMOUNT_UPLOADED:
+        case TR_AMOUNT_DOWNLOADED_SESSION:
+        case TR_AMOUNT_UPLOADED_SESSION:
+        case TR_AMOUNT_LEFT:
+        case TR_COMPLETED:
+        case TR_SIZE:
+        case TR_TOTAL_SIZE:
+        case TR_ETA:
+        case TR_SEEDS:
+        case TR_PEERS:
+        case TR_UPSPEED:
+        case TR_DLSPEED:
+        case TR_UPLIMIT:
+        case TR_DLLIMIT:
+        case TR_RATIO_LIMIT:
+        case TR_RATIO:
+        case TR_QUEUE_POSITION:
+        case TR_LAST_ACTIVITY:
+        case TR_AVAILABILITY:
+            return QVariant {Qt::AlignRight | Qt::AlignVCenter};
+        }
+    }
+
+    return {};
+}
+
 bool TransferListModel::setData(const QModelIndex &index, const QVariant &value, int role)
 {
     if (!index.isValid() || (role != Qt::DisplayRole)) return false;

src/gui/transferlistmodel.h

@@ -34,11 +34,11 @@
 #include <QColor>
 #include <QList>
 
+#include "base/bittorrent/torrenthandle.h"
+
 namespace BitTorrent
 {
     class InfoHash;
-    class TorrentHandle;
-    enum class TorrentState;
 }
 
 class TransferListModel : public QAbstractListModel
@@ -84,6 +84,12 @@ public:
         NB_COLUMNS
     };
 
+    enum DataRole
+    {
+        UnderlyingDataRole = Qt::UserRole,
+        AdditionalUnderlyingDataRole
+    };
+
     explicit TransferListModel(QObject *parent = nullptr);
 
     int rowCount(const QModelIndex &parent = {}) const override;
@@ -105,9 +111,12 @@ private slots:
     void handleTorrentsUpdated(const QVector<BitTorrent::TorrentHandle *> &torrents);
 
 private:
+    QString displayValue(const BitTorrent::TorrentHandle *torrent, int column) const;
+    QVariant internalValue(const BitTorrent::TorrentHandle *torrent, int column, bool alt = false) const;
+
     QList<BitTorrent::TorrentHandle *> m_torrentList;  // maps row number to torrent handle
     QHash<BitTorrent::TorrentHandle *, int> m_torrentMap;  // maps torrent handle to row number
-
+    const QHash<BitTorrent::TorrentState, QString> m_statusStrings;
     // row text colors
     QHash<BitTorrent::TorrentState, QColor> m_stateForegroundColors;
 };

src/gui/transferlistsortmodel.cpp

@@ -37,8 +37,9 @@
 #include "transferlistmodel.h"
 
 TransferListSortModel::TransferListSortModel(QObject *parent)
-    : QSortFilterProxyModel(parent)
+    : QSortFilterProxyModel {parent}
 {
+    QMetaType::registerComparators<BitTorrent::TorrentState>();
 }
 
 void TransferListSortModel::setStatusFilter(TorrentFilter::Type filter)
@@ -85,56 +86,100 @@ void TransferListSortModel::disableTrackerFilter()
 
 bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
 {
-    switch (sortColumn()) {
+    return lessThan_impl(left, right);
+}
+
+bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelIndex &right) const
+{
+    Q_ASSERT(left.column() == right.column());
+
+    const auto invokeLessThanForColumn = [this, &left, &right](const int column) -> bool
+    {
+        return lessThan_impl(left.sibling(left.row(), column), right.sibling(left.row(), column));
+    };
+
+    const int sortColumn = left.column();
+    const QVariant leftValue = left.data(TransferListModel::UnderlyingDataRole);
+    const QVariant rightValue = right.data(TransferListModel::UnderlyingDataRole);
+
+    switch (sortColumn) {
     case TransferListModel::TR_CATEGORY:
     case TransferListModel::TR_TAGS:
-    case TransferListModel::TR_NAME: {
-            const QVariant vL = left.data();
-            const QVariant vR = right.data();
-            if (!vL.isValid() || !vR.isValid() || (vL == vR))
-                return lowerPositionThan(left, right);
-
-            const int result = Utils::String::naturalCompare(vL.toString(), vR.toString(), Qt::CaseInsensitive);
-            return (result < 0);
-        }
+    case TransferListModel::TR_NAME:
+        if (!leftValue.isValid() || !rightValue.isValid() || (leftValue == rightValue))
+            return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION);
+        return (Utils::String::naturalCompare(leftValue.toString(), rightValue.toString(), Qt::CaseInsensitive) < 0);
+
+    case TransferListModel::TR_STATUS:
+        // QSortFilterProxyModel::lessThan() uses the < operator only for specific QVariant types
+        // so our custom type is outside that list.
+        // In this case QSortFilterProxyModel::lessThan() converts other types to QString and
+        // sorts them.
+        // Thus we can't use the code in the default label.
+        if (leftValue != rightValue)
+            return leftValue < rightValue;
+        return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION);
 
-    case TransferListModel::TR_STATUS: {
-            // QSortFilterProxyModel::lessThan() uses the < operator only for specific QVariant types
-            // so our custom type is outside that list.
-            // In this case QSortFilterProxyModel::lessThan() converts other types to QString and
-            // sorts them.
-            // Thus we can't use the code in the default label.
-            const auto leftValue = left.data().value<BitTorrent::TorrentState>();
-            const auto rightValue = right.data().value<BitTorrent::TorrentState>();
-            if (leftValue != rightValue)
-                return leftValue < rightValue;
+    case TransferListModel::TR_ADD_DATE:
+    case TransferListModel::TR_SEED_DATE:
+    case TransferListModel::TR_SEEN_COMPLETE_DATE: {
+            const QDateTime dateL = leftValue.toDateTime();
+            const QDateTime dateR = rightValue.toDateTime();
 
-            return lowerPositionThan(left, right);
+            if (dateL.isValid() && dateR.isValid()) {
+                if (dateL != dateR)
+                    return dateL < dateR;
+            }
+            else if (dateL.isValid()) {
+                return true;
+            }
+            else if (dateR.isValid()) {
+                return false;
+            }
         }
+        break;
 
-    case TransferListModel::TR_ADD_DATE:
-    case TransferListModel::TR_SEED_DATE:
-    case TransferListModel::TR_SEEN_COMPLETE_DATE:
-        return dateLessThan(sortColumn(), left, right, true);
+    case TransferListModel::TR_QUEUE_POSITION: {
+            // QVariant has comparators for all basic types
+            if ((leftValue > 0) || (rightValue > 0)) {
+                if ((leftValue > 0) && (rightValue > 0))
+                    return leftValue < rightValue;
 
-    case TransferListModel::TR_QUEUE_POSITION:
-        return lowerPositionThan(left, right);
+                return leftValue != 0;
+            }
+
+            // Sort according to TR_SEED_DATE
+            const QDateTime dateL = left.sibling(left.row(), TransferListModel::TR_SEED_DATE)
+                    .data(TransferListModel::UnderlyingDataRole).toDateTime();
+            const QDateTime dateR = right.sibling(right.row(), TransferListModel::TR_SEED_DATE)
+                    .data(TransferListModel::UnderlyingDataRole).toDateTime();
+
+            if (dateL.isValid() && dateR.isValid()) {
+                if (dateL != dateR)
+                    return dateL < dateR;
+            }
+            else if (dateL.isValid()) {
+                return false;
+            }
+            else if (dateR.isValid()) {
+                return true;
+            }
+        }
+        break;
 
     case TransferListModel::TR_SEEDS:
     case TransferListModel::TR_PEERS: {
-            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();
-
+            // QVariant has comparators for all basic types
             // Active peers/seeds take precedence over total peers/seeds.
-            if (leftActive != rightActive)
-                return (leftActive < rightActive);
+            if (leftValue != rightValue)
+                return (leftValue < rightValue);
 
-            if (leftTotal != rightTotal)
-                return (leftTotal < rightTotal);
+            const QVariant leftValueTotal = left.data(TransferListModel::AdditionalUnderlyingDataRole);
+            const QVariant rightValueTotal = right.data(TransferListModel::AdditionalUnderlyingDataRole);
+            if (leftValueTotal != rightValueTotal)
+                return (leftValueTotal < rightValueTotal);
 
-            return lowerPositionThan(left, right);
+            return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION);
         }
 
     case TransferListModel::TR_ETA: {
@@ -152,8 +197,10 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
             if (isActiveL != isActiveR)
                 return isActiveL;
 
-            const int queuePosL = left.sibling(left.row(), TransferListModel::TR_QUEUE_POSITION).data().toInt();
-            const int queuePosR = right.sibling(right.row(), TransferListModel::TR_QUEUE_POSITION).data().toInt();
+            const int queuePosL = left.sibling(left.row(), TransferListModel::TR_QUEUE_POSITION)
+                    .data(TransferListModel::UnderlyingDataRole).toInt();
+            const int queuePosR = right.sibling(right.row(), TransferListModel::TR_QUEUE_POSITION)
+                    .data(TransferListModel::UnderlyingDataRole).toInt();
             const bool isSeedingL = (queuePosL < 0);
             const bool isSeedingR = (queuePosR < 0);
             if (isSeedingL != isSeedingR) {
@@ -164,85 +211,36 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex
                 return isAscendingOrder;
             }
 
-            const qlonglong etaL = left.data().toLongLong();
-            const qlonglong etaR = right.data().toLongLong();
+            const qlonglong etaL = leftValue.toLongLong();
+            const qlonglong etaR = rightValue.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(TransferListModel::TR_SEED_DATE, left, right, true);
+                    return invokeLessThanForColumn(TransferListModel::TR_SEED_DATE);
 
                 return (queuePosL < queuePosR);
             }
-            if (!isInvalidL && !isInvalidR) {
+
+            if (!isInvalidL && !isInvalidR)
                 return (etaL < etaR);
-            }
 
             return !isInvalidL;
         }
 
-    case TransferListModel::TR_LAST_ACTIVITY: {
-            const int vL = left.data().toInt();
-            const int vR = right.data().toInt();
-
-            if (vL < 0) return false;
-            if (vR < 0) return true;
-
-            return (vL < vR);
-        }
-
-    case TransferListModel::TR_RATIO_LIMIT: {
-            const qreal vL = left.data().toReal();
-            const qreal vR = right.data().toReal();
-
-            if (vL < 0) return false;
-            if (vR < 0) return true;
+    case TransferListModel::TR_LAST_ACTIVITY:
+    case TransferListModel::TR_RATIO_LIMIT:
+        // QVariant has comparators for all basic types
+        if (leftValue < 0) return false;
+        if (rightValue < 0) return true;
 
-            return (vL < vR);
-        }
+        return (leftValue < rightValue);
 
     default:
-        if (left.data() != right.data())
+        if (leftValue != rightValue)
             return QSortFilterProxyModel::lessThan(left, right);
 
-        return lowerPositionThan(left, right);
-    }
-}
-
-bool TransferListSortModel::lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const
-{
-    // Sort according to TR_QUEUE_POSITION
-    const int queueL = left.sibling(left.row(), TransferListModel::TR_QUEUE_POSITION).data().toInt();
-    const int queueR = right.sibling(right.row(), TransferListModel::TR_QUEUE_POSITION).data().toInt();
-
-    if ((queueL > 0) || (queueR > 0)) {
-        if ((queueL > 0) && (queueR > 0))
-            return queueL < queueR;
-
-        return queueL != 0;
-    }
-
-    // Sort according to TR_SEED_DATE
-    return dateLessThan(TransferListModel::TR_SEED_DATE, left, right, false);
-}
-
-// Every time we compare QDateTimes we need a fallback comparison in case both
-// values are empty. This is a workaround for unstable sort in QSortFilterProxyModel
-// (detailed discussion in #2526 and #2158).
-bool TransferListSortModel::dateLessThan(const int dateColumn, const QModelIndex &left, const QModelIndex &right, bool sortInvalidInBottom) const
-{
-    const QDateTime dateL = left.sibling(left.row(), dateColumn).data().toDateTime();
-    const QDateTime dateR = right.sibling(right.row(), dateColumn).data().toDateTime();
-
-    if (dateL.isValid() && dateR.isValid()) {
-        if (dateL != dateR)
-            return dateL < dateR;
-    }
-    else if (dateL.isValid()) {
-        return sortInvalidInBottom;
-    }
-    else if (dateR.isValid()) {
-        return !sortInvalidInBottom;
+        return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION);
     }
 
     // Finally, sort by hash

src/gui/transferlistsortmodel.h

@@ -26,8 +26,7 @@
  * exception statement from your version.
  */
 
-#ifndef TRANSFERLISTSORTMODEL_H
-#define TRANSFERLISTSORTMODEL_H
+#pragma once
 
 #include <QSortFilterProxyModel>
 #include "base/torrentfilter.h"
@@ -37,9 +36,10 @@ class QStringList;
 class TransferListSortModel : public QSortFilterProxyModel
 {
     Q_OBJECT
+    Q_DISABLE_COPY(TransferListSortModel)
 
 public:
-    TransferListSortModel(QObject *parent = nullptr);
+    explicit TransferListSortModel(QObject *parent = nullptr);
 
     void setStatusFilter(TorrentFilter::Type filter);
     void setCategoryFilter(const QString &category);
@@ -51,13 +51,9 @@ public:
 
 private:
     bool lessThan(const QModelIndex &left, const QModelIndex &right) const override;
-    bool lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const;
-    bool dateLessThan(int dateColumn, const QModelIndex &left, const QModelIndex &right, bool sortInvalidInBottom) const;
     bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override;
     bool matchFilter(int sourceRow, const QModelIndex &sourceParent) const;
+    bool lessThan_impl(const QModelIndex &left, const QModelIndex &right) const;
 
-private:
     TorrentFilter m_filter;
 };
-
-#endif // TRANSFERLISTSORTMODEL_H