Enhancement - Command Line Parsing

- Fixed problem where a blank ID was considered valid.  For example,
  the command "task 1 -2" should use -2 as the description, but instead
  considered this to be the sequence 1,0,2.
- Replaced old validId calls with the new validSequence calls.
- A sequence has been redefined to be the first set of consecutive
  arguments that look like sequences.  Once broken by a non-sequence
  argument, all remaining args, even if they look like a sequence,
  are not considered part of the sequence.  This allows commands like
  "task append 1,3-5 Write 10 emails", where 10 is not part of the
  sequence because of the intervening "Write".
- Unit tests (parse.t.cpp) that exercise the parsing of sequences.
  Should probably be expanded to cover more.

src/parse.cpp

@@ -299,6 +299,9 @@ static bool validAttribute (
 ////////////////////////////////////////////////////////////////////////////////
 static bool validId (const std::string& input)
 {
+  if (input.length () == 0)
+    return false;
+
   for (size_t i = 0; i < input.length (); ++i)
     if (!::isdigit (input[i]))
       return false;
@@ -329,7 +332,6 @@ static bool validSequence (
 
       int id = ::atoi (range[0].c_str ());
       ids.push_back (id);
-//      std::cout << "# seq: " << id << std::endl;
       break;
 
     case 2:
@@ -344,10 +346,7 @@ static bool validSequence (
           return false;
 
         for (int i = low; i <= high; ++i)
-//        {
           ids.push_back (i);
-//          std::cout << "# seq: " << i << std::endl;
-//        }
       }
       break;
 
@@ -357,7 +356,7 @@ static bool validSequence (
     }
   }
 
-  return ids.size () > 1 ? true : false;
+  return ids.size () ? true : false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -448,8 +447,6 @@ bool validDuration (std::string& input)
 // -------      ----------------------------------
 // command      first non-id recognized argument
 //
-// id           ::= \d+
-//
 // substitution ::= "/" from "/" to "/g"
 //                | "/" from "/" to "/" ;
 //
@@ -459,8 +456,8 @@ bool validDuration (std::string& input)
 // attributes   ::= word ":" value
 //                | word ":"
 //
-// sequence     ::= id "," sequence
-//                | id "-" id ;
+// sequence     ::= \d+ "," sequence
+//                | \d+ "-" \d+ ;
 //
 // description  (whatever isn't one of the above)
 void parse (
@@ -471,6 +468,9 @@ void parse (
 {
   command = "";
 
+  bool foundSequence = false;
+  bool foundSomethingAfterSequence = false;
+
   std::string descCandidate = "";
   for (size_t i = 0; i < args.size (); ++i)
   {
@@ -488,22 +488,20 @@ void parse (
 
       // An id is the first argument found that contains all digits.
       if (lowerCase (command) != "add"  && // "add" doesn't require an ID
-          validSequence (arg, sequence))
+          validSequence (arg, sequence) &&
+          ! foundSomethingAfterSequence)
       {
+        foundSequence = true;
         foreach (id, sequence)
           task.addId (*id);
       }
 
-      else if (lowerCase (command) != "add" && // "add" doesn't require an ID
-               task.getId () == 0           &&
-               validId (arg))
-      {
-        task.setId (::atoi (arg.c_str ()));
-      }
-
       // Tags begin with + or - and contain arbitrary text.
       else if (validTag (arg))
       {
+        if (foundSequence)
+          foundSomethingAfterSequence = true;
+
         if (arg[0] == '+')
           task.addTag (arg.substr (1, std::string::npos));
         else if (arg[0] == '-')
@@ -514,6 +512,9 @@ void parse (
       // value.
       else if ((colon = arg.find (":")) != std::string::npos)
       {
+        if (foundSequence)
+          foundSomethingAfterSequence = true;
+
         std::string name  = lowerCase (arg.substr (0, colon));
         std::string value = arg.substr (colon + 1, std::string::npos);
 
@@ -536,12 +537,18 @@ void parse (
       // Substitution of description text.
       else if (validSubstitution (arg, from, to, global))
       {
+        if (foundSequence)
+          foundSomethingAfterSequence = true;
+
         task.setSubstitution (from, to, global);
       }
 
       // Command.
       else if (command == "")
       {
+        if (foundSequence)
+          foundSomethingAfterSequence = true;
+
         std::string l = lowerCase (arg);
         if (isCommand (l) && validCommand (l))
           command = l;
@@ -556,6 +563,9 @@ void parse (
       // Anything else is just considered description.
       else
       {
+        if (foundSequence)
+          foundSomethingAfterSequence = true;
+
         if (descCandidate.length ())
           descCandidate += " ";
         descCandidate += arg;

src/tests/.gitignore

@@ -5,3 +5,4 @@ date.t
 duration.t
 text.t
 autocomplete.t
+parse.t

src/tests/Makefile

@@ -1,4 +1,5 @@
-PROJECT = t.t tdb.t date.t duration.t t.benchmark.t text.t autocomplete.t
+PROJECT = t.t tdb.t date.t duration.t t.benchmark.t text.t autocomplete.t \
+          parse.t
 CFLAGS  = -I. -I.. -Wall -pedantic -ggdb3 -fno-rtti
 LFLAGS  = -L/usr/local/lib
 OBJECTS = ../TDB.o ../T.o ../parse.o ../text.o ../Date.o ../util.o ../Config.o
@@ -38,3 +39,6 @@ text.t: text.t.o $(OBJECTS) test.o
 autocomplete.t: autocomplete.t.o $(OBJECTS) test.o
 	g++ autocomplete.t.o $(OBJECTS) test.o $(LFLAGS) -o autocomplete.t
 
+parse.t: parse.t.o $(OBJECTS) test.o
+	g++ parse.t.o $(OBJECTS) test.o $(LFLAGS) -o parse.t
+

src/tests/parse.t.cpp

@@ -0,0 +1,134 @@
+////////////////////////////////////////////////////////////////////////////////
+// task - a command line task list manager.
+//
+// Copyright 2006 - 2009, 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
+//
+////////////////////////////////////////////////////////////////////////////////
+#include <iostream>
+#include "task.h"
+#include "test.h"
+
+////////////////////////////////////////////////////////////////////////////////
+int main (int argc, char** argv)
+{
+  UnitTest t (18);
+
+  std::vector <std::string> args;
+  std::string command;
+
+  Config conf;
+  conf.set ("dateformat", "m/d/Y");
+
+  {
+    T task;
+    split (args, "add foo", ' ');
+    parse (args, command, task, conf);
+    t.is (command,                "add", "(1) command found");
+    t.is (task.getId (),          0,     "(1) zero id on add");
+    t.is (task.getDescription (), "foo", "(1) correct description");
+  }
+
+  {
+    T task;
+    split (args, "delete 1,3-5,7", ' ');
+    parse (args, command, task, conf);
+    std::vector <int> sequence = task.getAllIds ();
+    t.is (sequence.size (), (size_t)5, "(2) sequence length");
+    if (sequence.size () == 5)
+    {
+      t.is (sequence[0], 1, "(2) sequence[0] == 1");
+      t.is (sequence[1], 3, "(2) sequence[1] == 3");
+      t.is (sequence[2], 4, "(2) sequence[2] == 4");
+      t.is (sequence[3], 5, "(2) sequence[3] == 5");
+      t.is (sequence[4], 7, "(2) sequence[4] == 7");
+    }
+    else
+    {
+      t.fail ("(2) sequence[0] == 1");
+      t.fail ("(2) sequence[1] == 3");
+      t.fail ("(2) sequence[2] == 4");
+      t.fail ("(2) sequence[3] == 5");
+      t.fail ("(2) sequence[4] == 7");
+    }
+  }
+
+  {
+    T task;
+    split (args, "delete 1,2 3,4", ' ');
+    parse (args, command, task, conf);
+    std::vector <int> sequence = task.getAllIds ();
+    t.is (sequence.size (), (size_t)4, "(3) sequence length");
+    if (sequence.size () == 4)
+    {
+      t.is (sequence[0], 1, "(3) sequence[0] == 1");
+      t.is (sequence[1], 2, "(3) sequence[1] == 2");
+      t.is (sequence[2], 3, "(3) sequence[2] == 3");
+      t.is (sequence[3], 4, "(3) sequence[3] == 4");
+    }
+    else
+    {
+      t.fail ("(3) sequence[0] == 1");
+      t.fail ("(3) sequence[1] == 2");
+      t.fail ("(3) sequence[2] == 3");
+      t.fail ("(3) sequence[3] == 4");
+    }
+  }
+
+  {
+    T task;
+    split (args, "1 There are 7 days in a week", ' ');
+    parse (args, command, task, conf);
+    std::vector <int> sequence = task.getAllIds ();
+    t.is (sequence.size (), (size_t)1, "(4) sequence length");
+    if (sequence.size () == 1)
+    {
+      t.is (sequence[0], 1, "(4) sequence[0] == 1");
+    }
+    else
+    {
+      t.fail ("(4) sequence[0] == 1");
+    }
+  }
+
+  {
+    T task;
+    args.clear ();
+    args.push_back ("1");
+    args.push_back ("4-123 is back-ordered");
+    parse (args, command, task, conf);
+    std::vector <int> sequence = task.getAllIds ();
+    t.is (sequence.size (), (size_t)1, "(5) sequence length");
+    if (sequence.size () == 1)
+    {
+      t.is (sequence[0], 1, "(5) sequence[0] == 1");
+    }
+    else
+    {
+      t.fail ("(5) sequence[0] == 1");
+    }
+  }
+  return 0;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+