Merge branch 'sort' into 1.9.3

src/Grid.cpp

@@ -369,15 +369,11 @@ Grid::Cell::operator std::string () const
   switch (mType)
   {
   case CELL_BOOL:   return mBool ? "true" : "false"; // TODO i18n
-  case CELL_CHAR:   sprintf (s, "%c", mChar);
+  case CELL_CHAR:   sprintf (s, "%c", mChar);   return std::string (s);
-                    return std::string (s);
+  case CELL_INT:    sprintf (s, "%d", mInt);    return std::string (s);
-  case CELL_INT:    sprintf (s, "%d", mInt);
+  case CELL_FLOAT:  sprintf (s, "%f", mFloat);  return std::string (s);
-                    return std::string (s);
+  case CELL_DOUBLE: sprintf (s, "%f", mDouble); return std::string (s);
-  case CELL_FLOAT:  sprintf (s, "%f", mFloat);
+  case CELL_STRING:                             return mString;
-                    return std::string (s);
-  case CELL_DOUBLE: sprintf (s, "%f", mDouble);
-                    return std::string (s);
-  case CELL_STRING: return mString;
   }
 
   return std::string ("");

src/Table.cpp

@@ -55,6 +55,7 @@
 #include "Context.h"
 
 extern Context context;
+Table* table = NULL;
 
 ////////////////////////////////////////////////////////////////////////////////
 Table::Table ()
@@ -591,239 +592,192 @@ int Table::columnCount ()
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// Combsort11, with O(n log n) average, O(n log n) worst case performance.
+// Essentially a static implementation of a dynamic operator<.
-//
+bool sort_compare (int left, int right)
-// function combsort11(array input)
-//     gap := input.size
-//
-//     loop until gap <= 1 and swaps = 0
-//         if gap > 1
-//             gap := gap / 1.3
-//             if gap = 10 or gap = 9
-//                 gap := 11
-//             end if
-//         end if
-//
-//         i := 0
-//         swaps := 0
-//
-//         loop until i + gap >= input.size
-//             if input[i] > input[i+gap]
-//                 swap(input[i], input[i+gap])
-//                 swaps := 1
-//             end if
-//             i := i + 1
-//         end loop
-//
-//     end loop
-// end function
-
-#define SWAP \
-   { \
-     int temp = order[r]; \
-     order[r] = order[r + gap]; \
-     order[r + gap] = temp; \
-     swaps = 1; \
-   }
-void Table::sort (std::vector <int>& order)
 {
-  int gap = order.size ();
+  for (size_t c = 0; c < table->mSortColumns.size (); ++c)
-  int swaps = 1;
-
-  while (gap > 1 || swaps > 0)
   {
-    if (gap > 1)
+    int column = table->mSortColumns[c];
-    {
+    Table::order sort_type = table->mSortOrder[column];
-      gap = (int) ((float)gap / 1.3);
-      if (gap == 10 || gap == 9)
-        gap = 11;
-    }
 
-    int r = 0;
+    Grid::Cell* cell_left  = table->mData.byRow (left,  column);
-    swaps = 0;
+    Grid::Cell* cell_right = table->mData.byRow (right, column);
 
-    while (r + gap < (int) order.size ())
+    // nothing < something.
+    if (cell_left == NULL && cell_right != NULL)
+      return (sort_type == Table::ascendingNumeric ||
+              sort_type == Table::ascendingCharacter ||
+              sort_type == Table::ascendingPriority ||
+              sort_type == Table::ascendingDate ||
+              sort_type == Table::ascendingDueDate ||
+              sort_type == Table::ascendingPeriod) ? true : false;
+
+    // something > nothing.
+    if (cell_left != NULL && cell_right == NULL)
+      return (sort_type == Table::ascendingNumeric ||
+              sort_type == Table::ascendingCharacter ||
+              sort_type == Table::ascendingPriority ||
+              sort_type == Table::ascendingDate ||
+              sort_type == Table::ascendingDueDate ||
+              sort_type == Table::ascendingPeriod) ? false : true;
+
+    // Equally NULL - next column.
+    if (cell_left == NULL && cell_right == NULL)
+      continue;
+
+    // Equal - next column
+    if (cell_left && cell_right && *cell_left == *cell_right)
+      continue;
+
+    // Differing data - do a proper comparison.
+    if (cell_left && cell_right)
     {
-      bool keepScanning = true;
+      switch (sort_type)
-      for (size_t c = 0; keepScanning && c < mSortColumns.size (); ++c)
       {
-        keepScanning = false;
+      case Table::ascendingNumeric:
+        return (float)*cell_left < (float)*cell_right ? true : false;
+        break;
 
-        Grid::Cell* left  = mData.byRow (order[r],       mSortColumns[c]);
+      case Table::descendingNumeric:
-        Grid::Cell* right = mData.byRow (order[r + gap], mSortColumns[c]);
+        return (float)*cell_left < (float)*cell_right ? false : true;
+        break;
 
-        // Data takes precedence over missing data.
+      case Table::ascendingCharacter:
-        if (left == NULL && right != NULL)
+        return (std::string)*cell_left < (std::string)*cell_right ? true : false;
+        break;
+
+      case Table::descendingCharacter:
+        return (std::string)*cell_left < (std::string)*cell_right ? false : true;
+        break;
+
+      case Table::ascendingDate:
         {
-          SWAP
+          // something > nothing.
-          break;
+          if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
-        }
+            return false;
 
-        // No data - try comparing the next column.
+          // nothing < something.
-        else if (left == NULL && right == NULL)
+          else if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
-        {
+            return true;
-          keepScanning = true;
-        }
 
-        // Identical data - try comparing the next column.
+          else
-        else if (left && right && *left == *right)
-        {
-          keepScanning = true;
-        }
-
-        // Differing data - do a proper comparison.
-        else if (left && right && *left != *right)
-        {
-          switch (mSortOrder[mSortColumns[c]])
           {
-          case ascendingNumeric:
+            Date dl ((std::string)*cell_left, table->mDateFormat);
-            if ((float)*left > (float)*right)
+            Date dr ((std::string)*cell_right, table->mDateFormat);
-              SWAP
+            return dl < dr ? true : false;
-            break;
-
-          case descendingNumeric:
-            if ((float)*left < (float)*right)
-              SWAP
-            break;
-
-          case ascendingCharacter:
-            if ((std::string)*left > (std::string)*right)
-              SWAP
-            break;
-
-          case descendingCharacter:
-            if ((std::string)*left < (std::string)*right)
-              SWAP
-            break;
-
-          case ascendingDate:
-            {
-              if (*left == *right)
-                break;
-
-              if ((std::string)*left != "" && (std::string)*right == "")
-                break;
-
-              else if ((std::string)*left == "" && (std::string)*right != "")
-                SWAP
-
-              else
-              {
-                Date dl ((std::string)*left, mDateFormat);
-                Date dr ((std::string)*right, mDateFormat);
-                if (dl > dr)
-                  SWAP
-              }
-            }
-            break;
-
-          case descendingDate:
-            {
-              if (*left == *right)
-                break;
-
-              if ((std::string)*left != "" && (std::string)*right == "")
-                break;
-
-              else if ((std::string)*left == "" && (std::string)*right != "")
-                SWAP
-
-              else
-              {
-                Date dl ((std::string)*left, mDateFormat);
-                Date dr ((std::string)*right, mDateFormat);
-                if (dl < dr)
-                  SWAP
-              }
-            }
-            break;
-
-          case ascendingDueDate:
-            {
-              if ((std::string)*left != "" && (std::string)*right == "")
-                break;
-
-              else if ((std::string)*left == "" && (std::string)*right != "")
-                SWAP
-
-              else
-              {
-                std::string format = context.config.get ("report." + mReportName + ".dateformat");
-                if (format == "")
-                  format = context.config.get ("dateformat.report");
-                if (format == "")
-                  format = context.config.get ("dateformat");
-
-                Date dl ((std::string)*left,  format);
-                Date dr ((std::string)*right, format);
-                if (dl > dr)
-                  SWAP
-              }
-            }
-            break;
-
-          case descendingDueDate:
-            {
-              if ((std::string)*left != "" && (std::string)*right == "")
-                break;
-
-              else if ((std::string)*left == "" && (std::string)*right != "")
-                SWAP
-
-              else
-              {
-                std::string format = context.config.get ("report." + mReportName + ".dateformat");
-                if (format == "")
-                  format = context.config.get ("dateformat.report");
-                if (format == "")
-                  format = context.config.get ("dateformat");
-
-                Date dl ((std::string)*left,  format);
-                Date dr ((std::string)*right, format);
-                if (dl < dr)
-                  SWAP
-              }
-            }
-            break;
-
-          case ascendingPriority:
-            if (((std::string)*left == ""  && (std::string)*right  != "")  ||
-                ((std::string)*left == "M" && (std::string)*right  == "L") ||
-                ((std::string)*left == "H" && ((std::string)*right == "L"  || (std::string)*right == "M")))
-              SWAP
-            break;
-
-          case descendingPriority:
-            if (((std::string)*left == ""  && (std::string)*right  != "")                                 ||
-                ((std::string)*left == "L" && ((std::string)*right == "M" || (std::string)*right == "H")) ||
-                ((std::string)*left == "M" && (std::string)*right  == "H"))
-              SWAP
-            break;
-
-          case ascendingPeriod:
-            if ((std::string)*left == "" && (std::string)*right != "")
-              break;
-            else if ((std::string)*left != "" && (std::string)*right == "")
-              SWAP
-            else if (Duration ((std::string)*left) > Duration ((std::string)*right))
-              SWAP
-            break;
-
-          case descendingPeriod:
-            if ((std::string)*left != "" && (std::string)*right == "")
-              break;
-            else if ((std::string)*left == "" && (std::string)*right != "")
-              SWAP
-            else if (Duration ((std::string)*left) < Duration ((std::string)*right))
-              SWAP
-            break;
           }
         }
-      }
+        break;
 
-      ++r;
+      case Table::descendingDate:
+        {
+          // something > nothing.
+          if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
+            return true;
+
+          // nothing < something.
+          else if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
+            return false;
+
+          else
+          {
+            Date dl ((std::string)*cell_left, table->mDateFormat);
+            Date dr ((std::string)*cell_right, table->mDateFormat);
+            return dl < dr ? false : true;
+          }
+        }
+        break;
+
+      case Table::ascendingDueDate:
+        {
+          // something > nothing.
+          if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
+            return false;
+
+          // nothing < something.
+          else if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
+            return true;
+
+          else
+          {
+            std::string format = context.config.get ("report." + table->mReportName + ".dateformat");
+            if (format == "")
+              format = context.config.get ("dateformat.report");
+            if (format == "")
+              format = context.config.get ("dateformat");
+
+            Date dl ((std::string)*cell_left,  format);
+            Date dr ((std::string)*cell_right, format);
+            return dl < dr ? true : false;
+          }
+        }
+        break;
+
+      case Table::descendingDueDate:
+        {
+          // something > nothing.
+          if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
+            return true;
+
+          // nothing < something.
+          else if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
+            return false;
+
+          else
+          {
+            std::string format = context.config.get ("report." + table->mReportName + ".dateformat");
+            if (format == "")
+              format = context.config.get ("dateformat.report");
+            if (format == "")
+              format = context.config.get ("dateformat");
+
+            Date dl ((std::string)*cell_left,  format);
+            Date dr ((std::string)*cell_right, format);
+            return dl < dr ? false : true;
+          }
+        }
+        break;
+
+      case Table::ascendingPriority:
+         if (((std::string)*cell_left == ""  && (std::string)*cell_right  != "")                                      ||
+            ((std::string)*cell_left  == "L" && ((std::string)*cell_right == "M" || (std::string)*cell_right == "H")) ||
+            ((std::string)*cell_left  == "M" && (std::string)*cell_right  == "H"))
+          return true;
+        else
+          return false;
+        break;
+
+      case Table::descendingPriority:
+        if (((std::string)*cell_left == ""  && (std::string)*cell_right  != "")  ||
+            ((std::string)*cell_left == "M" && (std::string)*cell_right  == "L") ||
+            ((std::string)*cell_left == "H" && ((std::string)*cell_right == "L"  || (std::string)*cell_right == "M")))
+         return true;
+        else
+          return false;
+        break;
+
+      case Table::ascendingPeriod:
+        if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
+          return true;
+        else if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
+          return false;
+        else if (Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right))
+          return true;
+        break;
+
+      case Table::descendingPeriod:
+        if ((std::string)*cell_left != "" && (std::string)*cell_right == "")
+          return false;
+        else if ((std::string)*cell_left == "" && (std::string)*cell_right != "")
+          return true;
+        else if (Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right))
+          return false;
+        break;
+      }
     }
   }
+
+  return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -868,7 +822,10 @@ const std::string Table::render (int maxrows /* = 0 */, int maxlines /* = 0 */)
 
   // Only sort if necessary.
   if (mSortColumns.size ())
-    sort (order);
+  {
+    table = this;  // Substitute for 'this' in the static 'sort_compare'.
+    std::sort (order.begin (), order.end (), sort_compare);
+  }
 
   // If a non-zero maxrows is specified, then it limits the number of rows of
   // the table that are rendered.

src/Table.h

@@ -35,6 +35,8 @@
 
 class Table
 {
+  friend bool sort_compare (int, int);
+
 public:
   enum just   {left, center, right};
   enum order  {ascendingNumeric,
@@ -101,7 +103,6 @@ private:
            const std::string formatHeader (const int, const int, const int);
            const std::string formatHeaderDashedUnderline (const int, const int, const int);
            void formatCell (const int, const int, const int, const int, const int, std::vector <std::string>&, std::string&);
-           void sort (std::vector <int>&);
 
 private:
   std::vector <std::string> mColumns;

src/tests/sorting.t

@@ -28,7 +28,7 @@
 
 use strict;
 use warnings;
-use Test::More tests => 7;
+use Test::More tests => 97;
 
 # Create the rc file.
 if (open my $fh, '>', 'sorting.rc')
@@ -38,19 +38,146 @@ if (open my $fh, '>', 'sorting.rc')
   ok (-r 'sorting.rc', 'Created sorting.rc');
 }
 
-# Test sort order for most reports (including list) of:
+# Test assorted sort orders.
-#   due+,priority-,active+,project+
+
-qx{../task rc:sorting.rc add due:eow priority:H project:A due:tomorrow one};
+qx{../task rc:sorting.rc add priority:H project:A due:yesterday one};
-qx{../task rc:sorting.rc add due:eow priority:H project:B due:tomorrow two};
+qx{../task rc:sorting.rc add priority:M project:B due:today     two};
-qx{../task rc:sorting.rc add due:eow priority:H project:C due:tomorrow three};
+qx{../task rc:sorting.rc add priority:L project:C due:tomorrow  three};
-qx{../task rc:sorting.rc add due:eow priority:H project:D due:tomorrow four};
+qx{../task rc:sorting.rc add priority:H project:C due:today     four};
-my $output = qx{../task rc:sorting.rc list};
-like ($output, qr/one.+two.+three.+four/ms, 'no active task sorting');
 
 qx{../task rc:sorting.rc start 1};
 qx{../task rc:sorting.rc start 3};
-$output = qx{../task rc:sorting.rc list};
+
-like ($output, qr/one.+three.+two.+four/ms, 'active task sorting');
+# pri:H pro:C   due:today     four
+# pri:H pro:A * due:yesterday one
+# pri:M pro:B   due:today     two
+# pri:L pro:C * due:tomorrow  three
+
+my %tests =
+(
+  # Single sort column.
+  'priority-'                       => '(?:one.+four|four.+one).+two.+three',
+  'priority+'                       => 'three.+two.+(?:one.+four|four.+one)',
+  'project-'                        => '(?:three.+four|four.+three).+two.+one',
+  'project+'                        => 'one.+two.+(?:three.+four|four.+three)',
+  'active-'                         => '(?:one.+three|three.+one).+(?:two.+four|four.+two)',
+  'active+'                         => '(?:two.+four|four.+two).+(?:one.+three|three.+one)',
+  'due-'                            => 'three.+(?:two.+four|four.+two).+one',
+  'due+'                            => 'one.+(?:two.+four|four.+two).+three',
+  'description-'                    => 'two.+three.+one.+four',
+  'description+'                    => 'four.+one.+three.+two',
+
+  # Two sort columns.
+  'priority-,project-'              => 'four.+one.+two.+three',
+  'priority-,project+'              => 'one.+four.+two.+three',
+  'priority+,project-'              => 'three.+two.+four.+one',
+  'priority+,project+'              => 'three.+two.+one.+four',
+
+  'priority-,active-'               => 'one.+four.+two.+three',
+  'priority-,active+'               => 'four.+one.+two.+three',
+  'priority+,active-'               => 'three.+two.+one.+four',
+  'priority+,active+'               => 'three.+two.+four.+one',
+
+  'priority-,due-'                  => 'four.+one.+two.+three',
+  'priority-,due+'                  => 'one.+four.+two.+three',
+  'priority+,due-'                  => 'three.+two.+four.+one',
+  'priority+,due+'                  => 'three.+two.+one.+four',
+
+  'priority-,description-'          => 'one.+four.+two.+three',
+  'priority-,description+'          => 'four.+one.+two.+three',
+  'priority+,description-'          => 'three.+two.+one.+four',
+  'priority+,description+'          => 'three.+two.+four.+one',
+
+  'project-,priority-'              => 'four.+three.+two.+one',
+  'project-,priority+'              => 'three.+four.+two.+one',
+  'project+,priority-'              => 'one.+two.+four.+three',
+  'project+,priority+'              => 'one.+two.+three.+four',
+
+  'project-,active-'                => 'three.+four.+two.+one',
+  'project-,active+'                => 'four.+three.+two.+one',
+  'project+,active-'                => 'one.+two.+three.+four',
+  'project+,active+'                => 'one.+two.+four.+three',
+
+  'project-,due-'                   => 'three.+four.+two.+one',
+  'project-,due+'                   => 'four.+three.+two.+one',
+  'project+,due-'                   => 'one.+two.+three.+four',
+  'project+,due+'                   => 'one.+two.+four.+three',
+
+  'project-,description-'           => 'three.+four.+two.+one',
+  'project-,description+'           => 'four.+three.+two.+one',
+  'project+,description-'           => 'one.+two.+three.+four',
+  'project+,description+'           => 'one.+two.+four.+three',
+
+  'active-,priority-'               => 'one.+three.+four.+two',
+  'active-,priority+'               => 'three.+one.+two.+four',
+  'active+,priority-'               => 'four.+two.+one.+three',
+  'active+,priority+'               => 'two.+four.+three.+one',
+
+  'active-,project-'                => 'three.+one.+four.+two',
+  'active-,project+'                => 'one.+three.+two.+four',
+  'active+,project-'                => 'four.+two.+three.+one',
+  'active+,project+'                => 'two.+four.+one.+three',
+
+  'active-,due-'                    => 'three.+one.+(?:four.+two|two.+four)',
+  'active-,due+'                    => 'one.+three.+(?:four.+two|two.+four)',
+  'active+,due-'                    => '(?:four.+two|two.+four).+three.+one',
+  'active+,due+'                    => '(?:four.+two|two.+four).+one.+three',
+
+  'active-,description-'            => 'three.+one.+two.+four',
+  'active-,description+'            => 'one.+three.+four.+two',
+  'active+,description-'            => 'two.+four.+three.+one',
+  'active+,description+'            => 'four.+two.+one.+three',
+
+  'due-,priority-'                  => 'three.+four.+two.+one',
+  'due-,priority+'                  => 'three.+two.+four.+one',
+  'due+,priority-'                  => 'one.+four.+two.+three',
+  'due+,priority+'                  => 'one.+two.+four.+three',
+
+  'due-,project-'                   => 'three.+four.+two.+one',
+  'due-,project+'                   => 'three.+two.+four.+one',
+  'due+,project-'                   => 'one.+four.+two.+three',
+  'due+,project+'                   => 'one.+two.+four.+three',
+
+  'due-,active-'                    => 'three.+(?:four.+two|two.+four).+one',
+  'due-,active+'                    => 'three.+(?:four.+two|two.+four).+one',
+  'due+,active-'                    => 'one.+(?:four.+two|two.+four).+three',
+  'due+,active+'                    => 'one.+(?:four.+two|two.+four).+three',
+
+  'due-,description-'               => 'three.+two.+four.+one',
+  'due-,description+'               => 'three.+four.+two.+one',
+  'due+,description-'               => 'one.+two.+four.+three',
+  'due+,description+'               => 'one.+four.+two.+three',
+
+  'description-,priority-'          => 'two.+three.+one.+four',
+  'description-,priority+'          => 'two.+three.+one.+four',
+  'description+,priority-'          => 'four.+one.+three.+two',
+  'description+,priority+'          => 'four.+one.+three.+two',
+
+  'description-,project-'           => 'two.+three.+one.+four',
+  'description-,project+'           => 'two.+three.+one.+four',
+  'description+,project-'           => 'four.+one.+three.+two',
+  'description+,project+'           => 'four.+one.+three.+two',
+
+  'description-,active-'            => 'two.+three.+one.+four',
+  'description-,active+'            => 'two.+three.+one.+four',
+  'description+,active-'            => 'four.+one.+three.+two',
+  'description+,active+'            => 'four.+one.+three.+two',
+
+  'description-,due-'               => 'two.+three.+one.+four',
+  'description-,due+'               => 'two.+three.+one.+four',
+  'description+,due-'               => 'four.+one.+three.+two',
+  'description+,due+'               => 'four.+one.+three.+two',
+
+  # Four sort columns.
+  'active+,project+,due+,priority+' => 'two.+four.+one.+three',
+  'project+,due+,priority+,active+' => 'one.+two.+four.+three',
+);
+
+for my $sort (sort keys %tests)
+{
+  my $output = qx{../task rc:sorting.rc rc.report.list.sort:${sort} list};
+  like ($output, qr/$tests{$sort}/ms, "sort:${sort}");
+}
 
 # Cleanup.
 unlink 'pending.data';