Merge e9bc4bea4d into 821a6816ef

As discussed in #234 and with @mikedld, the sorting code in the Qt frontend
needed a refactor.

This change proposes handling magnet transfers as a separate sorting category.
This means that magnets will always appear at the bottom (or top with reversed
sort order) of the list.

Sorting by Name, Size, Age, ID, Queue, ETA & Ratio remains straight forward.

Sorting by Progress sorts in the following order: download percentage, ratio,
queue position.

Sorting by Activity will first sort by activity, transfer speeds & peers, then
fallback to sorting by Progress.

Sorting by State now behaves the same as sorting by Activity. I'd like to
propose dropping this sorting mode, as it's now implicitly handled for sorting by
either Progress or Activity:

Transfers with errors come first, followed by finished transfers, active
transfers, paused items and eventually magnets.

CONTRIBUTING.md

@@ -41,7 +41,7 @@ On macOS, Transmission is usually built with Xcode. Everywhere else, it's CMake
 - Prefer `enum class` over `enum`
 - Prefer new-style headers, e.g. `<cstring>` over `<string.h>`
 - Fix any warnings in new code before merging
-- Run `./code-style.sh` on your code to ensure the whole codebase has consistent indentation.
+- Run `./code_style.sh` on your code to ensure the whole codebase has consistent indentation.
 
 Note that Transmission existed in C for over a decade and those idioms don't change overnight. "Follow the C++ core guidelines" can be difficult when working with older code, and the maintainers will understand that when reviewing your PRs. :smiley:
 

libtransmission/net.h

@@ -404,7 +404,7 @@ struct tr_socket_address
 };
 
 template<>
-class std::hash<tr_socket_address>
+struct std::hash<tr_socket_address>
 {
 public:
     std::size_t operator()(tr_socket_address const& socket_address) const noexcept

qt/TorrentFilter.cc

@@ -75,6 +75,26 @@ void TorrentFilter::refilter()
 ****
 ***/
 
+namespace
+{
+int compareState(Torrent const* left, Torrent const* right)
+{
+    if (auto const val = tr_compare_3way(left->hasError(), right->hasError()); val != 0)
+    {
+        return val;
+    }
+    if (auto const val = tr_compare_3way(left->isFinished(), right->isFinished()); val != 0)
+    {
+        return val;
+    }
+    if (auto const val = -tr_compare_3way(left->isPaused(), right->isPaused()); val != 0)
+    {
+        return val;
+    }
+    return -tr_compare_3way(!left->hasMetadata(), !right->hasMetadata());
+}
+} // namespace
+
 bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right) const
 {
     int val = 0;
@@ -84,37 +104,30 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
     switch (prefs_.get<SortMode>(Prefs::SORT_MODE).mode())
     {
     case SortMode::SORT_BY_QUEUE:
-        if (val == 0)
-        {
-            val = -tr_compare_3way(a->queuePosition(), b->queuePosition());
-        }
+        val = -tr_compare_3way(a->queuePosition(), b->queuePosition());
 
         break;
 
     case SortMode::SORT_BY_SIZE:
-        if (val == 0)
-        {
-            val = tr_compare_3way(a->sizeWhenDone(), b->sizeWhenDone());
-        }
+        val = tr_compare_3way(a->sizeWhenDone(), b->sizeWhenDone());
 
         break;
 
     case SortMode::SORT_BY_AGE:
-        if (val == 0)
-        {
-            val = tr_compare_3way(a->dateAdded(), b->dateAdded());
-        }
+        val = tr_compare_3way(a->dateAdded(), b->dateAdded());
 
         break;
 
     case SortMode::SORT_BY_ID:
-        if (val == 0)
-        {
-            val = tr_compare_3way(a->id(), b->id());
-        }
+        val = tr_compare_3way(a->id(), b->id());
 
         break;
 
+    case SortMode::SORT_BY_ETA:
+        val = a->compareETA(*b);
+
+        [[fallthrough]];
+
     case SortMode::SORT_BY_ACTIVITY:
         if (val == 0)
         {
@@ -124,8 +137,8 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
         if (val == 0)
         {
             val = tr_compare_3way(
-                a->peersWeAreUploadingTo() + a->webseedsWeAreDownloadingFrom(),
-                b->peersWeAreUploadingTo() + b->webseedsWeAreDownloadingFrom());
+                a->peersWeAreUploadingTo() + a->peersWeAreDownloadingFrom() + a->webseedsWeAreDownloadingFrom(),
+                b->peersWeAreUploadingTo() + b->peersWeAreDownloadingFrom() + b->webseedsWeAreDownloadingFrom());
         }
 
         [[fallthrough]];
@@ -133,22 +146,7 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
     case SortMode::SORT_BY_STATE:
         if (val == 0)
         {
-            val = -tr_compare_3way(a->isPaused(), b->isPaused());
-        }
-
-        if (val == 0)
-        {
-            val = tr_compare_3way(a->getActivity(), b->getActivity());
-        }
-
-        if (val == 0)
-        {
-            val = -tr_compare_3way(a->queuePosition(), b->queuePosition());
-        }
-
-        if (val == 0)
-        {
-            val = tr_compare_3way(a->hasError(), b->hasError());
+            val = compareState(a, b);
         }
 
         [[fallthrough]];
@@ -161,12 +159,12 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
 
         if (val == 0)
         {
-            val = tr_compare_3way(a->percentComplete(), b->percentComplete());
+            val = a->compareSeedProgress(*b);
         }
 
         if (val == 0)
         {
-            val = a->compareSeedProgress(*b);
+            val = tr_compare_3way(a->getActivity(), b->getActivity());
         }
 
         if (val == 0)
@@ -174,9 +172,10 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
             val = -tr_compare_3way(a->queuePosition(), b->queuePosition());
         }
 
-        [[fallthrough]];
+        break;
 
     case SortMode::SORT_BY_RATIO:
+        val = -tr_compare_3way(!a->hasMetadata(), !b->hasMetadata());
         if (val == 0)
         {
             val = a->compareRatio(*b);
@@ -184,14 +183,12 @@ bool TorrentFilter::lessThan(QModelIndex const& left, QModelIndex const& right)
 
         break;
 
-    case SortMode::SORT_BY_ETA:
-        if (val == 0)
-        {
-            val = a->compareETA(*b);
-        }
-
+    case SortMode::SORT_BY_NAME:
+        // nothing to do: sorting by name is done after the switch
         break;
 
+        // TODO(coeur): SORT_BY_TRACKER
+
     default:
         break;
     }

utils/transmission-remote.1

@@ -150,7 +150,7 @@ Get a file list for the current torrent(s)
 .It Fl g Fl -get Ar all | file-index | files
 Mark file(s) for download.
 .Ar all
-marks all all of the torrent's files for downloading,
+marks all of the torrent's files for downloading,
 .Ar file-index
 adds a single file to the download list, and
 .Ar files