Bug #417 - Sorting by countdown_compact not working

- Added support for more varied durations when specifying recurring tasks,
  such as '3 mths' or '24 hrs'.
- Fixed bug #417, which caused sorting on countdown fields to be wrong
  (thanks to Michell Crane).
- Durations are now based on seconds, rather than days, and can accept/parse
  negative durations.

ChangeLog

@@ -25,9 +25,11 @@
    by specifying either 'limit:page' to a command, or 'report.xxx.limit:page'
    in a report specification (thanks to T. Charles Yun).
  + Improvements to the man pages (thanks to T. Charles Yun).
- + Modified the 'next' report to only display a page, by default.
+ + Modified the 'next' report to only display one page, by default.
  + Added feature #408, making it possible to delete annotations with the new
    denotate command and the provided description (thanks to Dirk Deimeke).
+ + Added support for more varied durations when specifying recurring tasks,
+   such as '3 mths' or '24 hrs'.
  + Fixed bug #406 so that task now includes command aliases in the _commands
    helper command used by shell completion scripts.
  + Fixed bug #211 - it was unclear which commands modify a task description.
@@ -37,6 +39,8 @@
    containing Unicode characters to fail (thanks to Michal Josífko).
  + Fixed bug #416, which caused sorting on a date to fail if the year was not
    included in the dateformat (thanks to Michelle Crane).
+ + Fixed bug #417, which caused sorting on countdown fields to be wrong
+   (thanks to Michell Crane).
 
 ------ old releases ------------------------------
 

src/Duration.cpp

@@ -36,13 +36,23 @@
 ////////////////////////////////////////////////////////////////////////////////
 Duration::Duration ()
 : mSecs (0)
+, mNegative (false)
 {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 Duration::Duration (time_t input)
 {
-  mSecs = input;
+  if (input < 0)
+  {
+    mSecs = -input;
+    mNegative = true;
+  }
+  else
+  {
+    mSecs = input;
+    mNegative = false;
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -61,7 +71,7 @@ Duration::operator time_t ()
 Duration::operator std::string ()
 {
   std::stringstream s;
-  s << mSecs;
+  s << (mNegative ? -mSecs : mSecs);
   return s.str ();
 }
 
@@ -69,7 +79,10 @@ Duration::operator std::string ()
 Duration& Duration::operator= (const Duration& other)
 {
   if (this != &other)
-    mSecs = other.mSecs;
+  {
+    mSecs     = other.mSecs;
+    mNegative = other.mNegative;
+  }
 
   return *this;
 }
@@ -81,30 +94,30 @@ std::string Duration::format () const
   float days = (float) mSecs / 86400.0;
 
   if (mSecs >= 86400 * 365)
-    sprintf (formatted, "%.1f yrs", (days / 365));
+    sprintf (formatted, "%s%.1f yrs", (mNegative ? "-" : ""), (days / 365));
   else if (mSecs > 86400 * 84)
-    sprintf (formatted, "%1d mth%s",
+    sprintf (formatted, "%s%1d mth%s",
-                         (int) (float) (days / 30.6),
+                        (mNegative ? "-" : ""),  (int) (float) (days / 30.6),
                         ((int) (float) (days / 30.6) == 1 ? "" : "s"));
   else if (mSecs > 86400 * 13)
-    sprintf (formatted, "%d wk%s",
+    sprintf (formatted, "%s%d wk%s",
-                         (int) (float) (days / 7.0),
+                        (mNegative ? "-" : ""),  (int) (float) (days / 7.0),
                         ((int) (float) (days / 7.0) == 1 ? "" : "s"));
   else if (mSecs >= 86400)
-    sprintf (formatted, "%d day%s",
+    sprintf (formatted, "%s%d day%s",
-                         (int) days,
+                        (mNegative ? "-" : ""),  (int) days,
                         ((int) days == 1 ? "" : "s"));
   else if (mSecs >= 3600)
-    sprintf (formatted, "%d hr%s",
+    sprintf (formatted, "%s%d hr%s",
-                         (int) (float) (mSecs / 3600),
+                        (mNegative ? "-" : ""),  (int) (float) (mSecs / 3600),
                         ((int) (float) (mSecs / 3600) == 1 ? "" : "s"));
   else if (mSecs >= 60)
-    sprintf (formatted, "%d min%s",
+    sprintf (formatted, "%s%d min%s",
-                         (int) (float) (mSecs / 60),
+                        (mNegative ? "-" : ""),  (int) (float) (mSecs / 60),
                         ((int) (float) (mSecs / 60) == 1 ? "" : "s"));
   else if (mSecs >= 1)
-    sprintf (formatted, "%d sec%s",
+    sprintf (formatted, "%s%d sec%s",
-                         (int) mSecs,
+                        (mNegative ? "-" : ""),  (int) mSecs,
                         ((int) mSecs == 1 ? "" : "s"));
   else
     strcpy (formatted, "-"); // no i18n
@@ -118,13 +131,13 @@ std::string Duration::formatCompact () const
   char formatted[24];
   float days = (float) mSecs / 86400.0;
 
-       if (mSecs >= 86400 * 365) sprintf (formatted, "%.1fy", (days / 365));
+       if (mSecs >= 86400 * 365) sprintf (formatted, "%s%.1fy", (mNegative ? "-" : ""), (days / 365));
-  else if (mSecs >= 86400 * 84)  sprintf (formatted, "%1dmo", (int) (float) (days / 30.6));
+  else if (mSecs >= 86400 * 84)  sprintf (formatted, "%s%1dmo", (mNegative ? "-" : ""), (int) (float) (days / 30.6));
-  else if (mSecs >= 86400 * 13)  sprintf (formatted, "%dwk",  (int) (float) (days / 7.0));
+  else if (mSecs >= 86400 * 13)  sprintf (formatted, "%s%dwk",  (mNegative ? "-" : ""), (int) (float) (days / 7.0));
-  else if (mSecs >= 86400)       sprintf (formatted, "%dd",   (int) days);
+  else if (mSecs >= 86400)       sprintf (formatted, "%s%dd",   (mNegative ? "-" : ""), (int) days);
-  else if (mSecs >= 3600)        sprintf (formatted, "%dh",   (int) (float) (mSecs / 3600));
+  else if (mSecs >= 3600)        sprintf (formatted, "%s%dh",   (mNegative ? "-" : ""), (int) (float) (mSecs / 3600));
-  else if (mSecs >= 60)          sprintf (formatted, "%dm",   (int) (float) (mSecs / 60));
+  else if (mSecs >= 60)          sprintf (formatted, "%s%dm",   (mNegative ? "-" : ""), (int) (float) (mSecs / 60));
-  else if (mSecs >= 1)           sprintf (formatted, "%ds",   (int) mSecs);
+  else if (mSecs >= 1)           sprintf (formatted, "%s%ds",   (mNegative ? "-" : ""), (int) mSecs);
   else                           strcpy (formatted, "-");
 
   return std::string (formatted);
@@ -133,13 +146,19 @@ std::string Duration::formatCompact () const
 ////////////////////////////////////////////////////////////////////////////////
 bool Duration::operator< (const Duration& other)
 {
-  return mSecs < other.mSecs;
+  long left  = (long) (      mNegative ?       -mSecs :       mSecs);
+  long right = (long) (other.mNegative ? -other.mSecs : other.mSecs);
+
+  return left < right;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 bool Duration::operator> (const Duration& other)
 {
-  return mSecs > other.mSecs;
+  long left  = (long) (      mNegative ?       -mSecs :       mSecs);
+  long right = (long) (other.mNegative ? -other.mSecs : other.mSecs);
+
+  return left > right;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -173,14 +192,18 @@ bool Duration::valid (const std::string& input) const
   if (autoComplete (lower_input, supported, matches) == 1)
     return true;
 
-  // Support \d+ \s? s|secs?|m|mins?|h|hrs?|d|days?|wks?|mo|mths?|y|yrs?|-
+  // Support \s+ -? \d+ \s? s|secs?|m|mins?|h|hrs?|d|days?|wks?|mo|mths?|y|yrs?|-
   // Note: Does not support a sign character.  That must be external to
   // Duration.
   Nibbler n (lower_input);
   int value;
+
+  n.skipAll (' ');
+  n.skip ('-');
+
   if (n.getUnsignedInt (value))
   {
-    n.skip (' ');
+    n.skipAll (' ');
 
          if (n.getLiteral ("yrs")  && n.depleted ()) return true;
     else if (n.getLiteral ("yr")   && n.depleted ()) return true;
@@ -259,16 +282,22 @@ void Duration::parse (const std::string& input)
     else if (found == "biannual" || found == "biyearly")  mSecs = 86400 * 730; // TODO i18n
   }
 
-  // Support \d+ \s? s|secs?|m|mins?|h|hrs?|d|days?|wks?|mo|mths?|y|yrs?|-
+  // Support -? \d+ \s? s|secs?|m|mins?|h|hrs?|d|days?|wks?|mo|mths?|y|yrs?|-
   // Note: Does not support a sign character.  That must be external to
   // Duration.
   else
   {
     Nibbler n (lower_input);
+
+    n.skipAll (' ');
+    mNegative = false;
+    if (n.skip ('-'))
+      mNegative = true;
+
     int value;
     if (n.getUnsignedInt (value))
     {
-      n.skip (' ');
+      n.skipAll (' ');
 
            if (n.getLiteral ("yrs")  && n.depleted ()) mSecs = value * 86400 * 365;
       else if (n.getLiteral ("yr")   && n.depleted ()) mSecs = value * 86400 * 365;

src/Duration.h

@@ -52,6 +52,7 @@ public:
 
 private:
   time_t mSecs;
+  bool mNegative;
 };
 
 #endif

src/custom.cpp

@@ -435,14 +435,7 @@ int runCustomReport (
           if (due.length ())
           {
             Date dt (::atoi (due.c_str ()));
-            time_t cntdwn = (time_t) (now - dt);
+            countdown = Duration (now - dt).format ();
-            Duration du (cntdwn < 0 ? -cntdwn : cntdwn);
-
-            if (cntdwn < 0)
-              countdown = std::string ("-") + du.format ();
-            else
-              countdown = du.format ();
-
             context.hooks.trigger ("format-countdown", "countdown", countdown);
             table.addCell (row, columnCount, countdown);
           }
@@ -464,14 +457,7 @@ int runCustomReport (
           if (due.length ())
           {
             Date dt (::atoi (due.c_str ()));
-            time_t cntdwn = (time_t) (now - dt);
+            countdown = Duration (now - dt).formatCompact ();
-            Duration du (cntdwn < 0 ? -cntdwn : cntdwn);
-
-            if (cntdwn < 0)
-              countdown = std::string ("-") + du.formatCompact ();
-            else
-              countdown = du.formatCompact ();
-
             context.hooks.trigger ("format-countdown_compact", "countdown_compact", countdown);
             table.addCell (row, columnCount, countdown);
           }
@@ -699,6 +685,12 @@ int runCustomReport (
                         Table::ascendingPeriod :
                         Table::descendingPeriod));
 
+      else if (column == "countdown" || column == "countdown_compact")
+        table.sortOn (columnIndex[column],
+                      (direction == '+' ?
+                        Table::descendingPeriod :   // Yes, these are flipped.
+                        Table::ascendingPeriod));   // Yes, these are flipped.
+
       else
         table.sortOn (columnIndex[column],
                       (direction == '+' ?

src/tests/bug.417.t

@@ -0,0 +1,66 @@
+#! /usr/bin/perl
+################################################################################
+## task - a command line task list manager.
+##
+## Copyright 2006 - 2010, Paul Beckingham.
+## All rights reserved.
+##
+## This program is free software; you can redistribute it and/or modify it under
+## the terms of the GNU General Public License as published by the Free Software
+## Foundation; either version 2 of the License, or (at your option) any later
+## version.
+##
+## This program is distributed in the hope that it will be useful, but WITHOUT
+## ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+## FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+## details.
+##
+## You should have received a copy of the GNU General Public License along with
+## this program; if not, write to the
+##
+##     Free Software Foundation, Inc.,
+##     51 Franklin Street, Fifth Floor,
+##     Boston, MA
+##     02110-1301
+##     USA
+##
+################################################################################
+
+use strict;
+use warnings;
+use Test::More tests => 7;
+
+# Create the rc file.
+if (open my $fh, '>', 'bug.rc')
+{
+  print $fh "data.location=.\n";
+  close $fh;
+  ok (-r 'bug.rc', 'Created bug.rc');
+}
+
+# Bug #417: Sorting by countdown_compact not working
+qx{../task rc:bug.rc add due:yesterday before};
+qx{../task rc:bug.rc add due:today     now};
+qx{../task rc:bug.rc add due:tomorrow  after};
+
+my $output = qx{../task rc:bug.rc rc.report.long.sort:countdown+ long};
+like ($output, qr/before.+now.+after/ms, 'rc.report.long.sort:countdown+ works');
+
+$output = qx{../task rc:bug.rc rc.report.long.sort:countdown- long};
+like ($output, qr/after.+now.+before/ms, 'rc.report.long.sort:countdown- works');
+
+# Cleanup.
+unlink 'pending.data';
+ok (!-r 'pending.data', 'Removed pending.data');
+
+unlink 'completed.data';
+ok (!-r 'completed.data', 'Removed completed.data');
+
+unlink 'undo.data';
+ok (!-r 'undo.data', 'Removed undo.data');
+
+unlink 'bug.rc';
+ok (!-r 'bug.rc', 'Removed bug.rc');
+
+exit 0;
+

src/tests/duration.t.cpp

@@ -48,7 +48,7 @@ int convertDuration (const std::string& input)
 
 int main (int argc, char** argv)
 {
-  UnitTest t (559);
+  UnitTest t (577);
 
   Duration d;
 
@@ -670,6 +670,36 @@ int main (int argc, char** argv)
   t.is (convertDuration ("10 day"),       10, "valid duration 10 day = 10");
   t.is (convertDuration ("10d"),          10, "valid duration 10d = 10");
 
+  try
+  {
+    Duration left, right;
+
+    // operator<
+    left = Duration ("1sec");   right = Duration ("2secs");  t.ok (left < right, "duration 1sec < 2secs");
+    left = Duration ("-2secs"); right = Duration ("-1sec");  t.ok (left < right, "duration -2secs < -1sec");
+    left = Duration ("1sec");   right = Duration ("1min");   t.ok (left < right, "duration 1sec < 1min");
+    left = Duration ("1min");   right = Duration ("1hr");    t.ok (left < right, "duration 1min < 1hr");
+    left = Duration ("1hr");    right = Duration ("1d");     t.ok (left < right, "duration 1hr < 1d");
+    left = Duration ("1d");     right = Duration ("1w");     t.ok (left < right, "duration 1d < 1w");
+    left = Duration ("1w");     right = Duration ("1mo");    t.ok (left < right, "duration 1w < 1mo");
+    left = Duration ("1mo");    right = Duration ("1q");     t.ok (left < right, "duration 1mo < 1q");
+    left = Duration ("1q");     right = Duration ("1y");     t.ok (left < right, "duration 1q < 1y");
+
+    // operator>
+    left = Duration ("2secs");  right = Duration ("1sec");   t.ok (left > right, "2sec > 1secs");
+    left = Duration ("-1sec");  right = Duration ("-2secs"); t.ok (left > right, "-1secs > -2sec");
+    left = Duration ("1min");   right = Duration ("1sec");   t.ok (left > right, "1min > 1sec");
+    left = Duration ("1hr");    right = Duration ("1min");   t.ok (left > right, "1hr > 1min");
+    left = Duration ("1d");     right = Duration ("1hr");    t.ok (left > right, "1d > 1hr");
+    left = Duration ("1w");     right = Duration ("1d");     t.ok (left > right, "1w > 1d");
+    left = Duration ("1mo");    right = Duration ("1w");     t.ok (left > right, "1mo > 1w");
+    left = Duration ("1q");     right = Duration ("1mo");    t.ok (left > right, "1q > 1mo");
+    left = Duration ("1y");     right = Duration ("1q");     t.ok (left > right, "1y > 1q");
+  }
+
+  catch (const std::string& e) { t.diag (e); }
+  catch (...) { t.diag ("Unknown error"); }
+
   return 0;
 }