Bug #972, #1018

- Fixed bug that accepted a recurrence duration of '7' as '7secs' instead
  of the intended '7days'.  It is now an error to omit the units.  Thanks to
  Vlad Zhivotnev, Stanley G.
- Modified unit tests to avoid a different error.

AUTHORS

@@ -146,4 +146,5 @@ suggestions:
   Martin U
   Christoph Lange
   Stephen Hay
-
+  Vlad Zhivotnev
+  Stanley G

ChangeLog

@@ -44,6 +44,8 @@ Bugs
  + Fixed bug #964, where the 'projects' command showed the wrong priority labels
    (thanks to Ali Mousavi).
  + Fixed bug #968, a typo in the 'edit' command text (thanks to Victor Roetman).
+ + Fixed bug #972, #1018, which caused a recurrence of "7" to be interpreted as
+   "7secs", instead of generating an error (thanks to Vlad Zhivotnev, Stanley G).
  + Fixed bug #973, including 'urgency' in the 'export' output (thanks to Andy
    Spiegl).
  + Fixed bug #986, so that the 'info' report uses the correct date format, also

src/Duration.cpp

@@ -135,11 +135,16 @@ Duration::Duration (const std::string& input)
 {
   if (digitsOnly (input))
   {
-    _secs = (time_t) strtol (input.c_str (), NULL, 10);
+    time_t value = (time_t) strtol (input.c_str (), NULL, 10);
-    _negative = false;
+    if (value == 0 || value > 60)
+    {
+      _secs = value;
+      _negative = false;
+      return;
+    }
   }
-  else
+
-    parse (input);
+  parse (input);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -362,8 +367,9 @@ bool Duration::valid (const std::string& input)
   n.getUntilEOS (units);
 
   // Non-trivial value with no units means the duration is specified in
-  // seconds, and therefore a time_t.  Consider it valid.
+  // seconds, and therefore a time_t.  Consider it valid provided it is >= 60.
-  if (value != 0.0 &&
+  if (value != 0.0  &&
+      value >= 60.0 &&
       units == "")
     return true;
 
@@ -401,13 +407,13 @@ void Duration::parse (const std::string& input)
     _negative = false;
 
   // If no units are provided, assume seconds.
-  if (n.depleted ())
+  std::string units;
+  if (n.depleted () && value >= 60)
   {
     _secs = (long) value;
     return;
   }
 
-  std::string units;
   n.getUntilEOS (units);
 
   // Auto complete against all supported durations.

src/commands/Command.cpp

@@ -545,10 +545,9 @@ void Command::modify_task (
             long l = (long) strtod (result.c_str (), NULL);
             if (labs (l) < 5 * 365 * 86400)
             {
-              Duration dur (result);
+              Duration dur (value);
               Date now;
               now += l;
-              //now += dur;
               task.set (name, now.toEpochString ());
             }
             else
@@ -562,7 +561,7 @@ void Command::modify_task (
           else if (name == "recur" ||
                    column->type () == "duration")
           {
-            // All values must be eval'd first.
+            // All values must be eval'd first, in this case, just to catch errors.
             A3 value_tokens;
             value_tokens.capture (value);
             value_tokens = value_tokens.postfix (value_tokens.tokenize (value_tokens));
@@ -571,8 +570,11 @@ void Command::modify_task (
             std::string result = e.evalExpression (task);
             context.debug (std::string ("Eval '") + value + "' --> '" + result + "'");
 
-            Duration d (result);
+            Duration d (value);
-            task.set (name, result);
+
+            // Deliberately storing the 'raw' value, which is necessary for
+            // durations like 'weekday'..
+            task.set (name, value);
           }
 
           // Need handling for numeric types, used by UDAs.

test/feature.1013.t

@@ -42,9 +42,9 @@ if (open my $fh, '>', 'outerr.rc')
 # error
 
 # Check that errors are sent to standard error
-my $stdout = qx{../src/task rc:outerr.rc add due:__ 2> /dev/null};
+my $stdout = qx{../src/task rc:outerr.rc add due:__ foo 2> /dev/null};
 unlike ($stdout, qr/^The duration '__' was not recognized as valid, with correct units like '3days'.$/ms, 'Errors are not sent to stdout');
-my $stderr = qx{../src/task rc:outerr.rc add due:__ 2>&1 >/dev/null};
+my $stderr = qx{../src/task rc:outerr.rc add due:__ bar 2>&1 >/dev/null};
 like ($stderr, qr/^The duration '__' was not recognized as valid, with correct units like '3days'.$/ms, 'Errors are sent to stderr');
 
 # Check that headers are sent to standard error