From 514c869aea422449f241b67a7f9e2ae00b0ab976 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tomasbabej@gmail.com>
Date: Sun, 13 Sep 2015 14:49:55 +0200
Subject: [PATCH] sort: Do not use Variant to sort UDA strings

String UDA defer the evaluation of the sort_compare method
to the '<' operator of the Variant class, since the logic for
comparing string UDAs (which can have custom sort orders) is
already implemented there.

However, since primary use case of Variant is the filter evaluation,
it handles trivial (empty) values in a manner which does not allow
for its direct usage as the comparison operator between sort UDAs.

When filtering, a trivial (empty) value should neither match
'uda < A' nor 'uda >= A'. However, when sorting tasks upon values of
this UDA, precisely one of the above expressions *must* be true.

This commit implements the UDA string sorting logic directly in
the sort_compare method.
---
 src/sort.cpp | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/sort.cpp b/src/sort.cpp
index 3ee64f503..f99ada61c 100644
--- a/src/sort.cpp
+++ b/src/sort.cpp
@@ -30,7 +30,6 @@
 #include <string>
 #include <stdlib.h>
 #include <Context.h>
-#include <Variant.h>
 #include <ISO8601.h>
 #include <Task.h>
 #include <text.h>
@@ -68,7 +67,6 @@ void sort_tasks (
 // require re-parsing.
 //
 // Essentially a static implementation of a dynamic operator<.
-// UDA string values delegate to Variant::operator<.
 static bool sort_compare (int left, int right)
 {
   std::string field;
@@ -213,20 +211,36 @@ static bool sort_compare (int left, int right)
         return ascending ? (left_real < right_real)
                          : (left_real > right_real);
       }
-      // UDA values of type 'string' are sorted by Variant::operator<.
-      // By setting 'source' to the UDA name, the comparison operator can use
-      // the custom sort order, if defined.
       else if (type == "string")
       {
-        Variant l ((*global_data)[left].get_ref  (field));
-        Variant r ((*global_data)[right].get_ref (field));
-        if (l == r)
+        const std::string left_string = (*global_data)[left].get_ref (field);
+        const std::string right_string = (*global_data)[right].get_ref (field);
+
+        if (left_string == right_string)
           continue;
 
-        l.source (field);
-        r.source (field);
-        return ascending ? (l < r) : (r < l);
+        // UDAs of the type string can have custom sort orders, which need to be considered.
+        auto order = Task::customOrder.find (field);
+        if (order != Task::customOrder.end ())
+        {
+          // Guaranteed to be found, because of ColUDA::validate ().
+          auto posLeft  = std::find (order->second.begin (), order->second.end (), left_string);
+          auto posRight = std::find (order->second.begin (), order->second.end (), right_string);
+          return ascending ? (posLeft < posRight) : (posLeft > posRight);
+        }
+        else
+        {
+          // Empty values are unconditionally last, if no custom order was specified.
+          if (left_string == "")
+            return false;
+          else if (right_string == "")
+            return true;
+
+          return ascending ? (left_string < right_string)
+                           : (left_string > right_string);
+        }
       }
+
       else if (type == "date")
       {
         const std::string& left_string  = (*global_data)[left].get_ref  (field);