From 0b14efbb76c6ad4a944299a89b5ab932f54f9ff8 Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Fri, 12 Jun 2009 01:45:42 -0400 Subject: [PATCH] Enhancements - validation - Implemented Cmd::isReadOnlyCommand. - Implemented Cmd::isWriteCommand. - Added unit tests for above. --- src/Att.cpp | 4 +- src/Cmd.cpp | 47 ++++++ src/Cmd.h | 3 + src/Context.cpp | 10 +- src/main.h | 11 +- src/tests/cmd.t.cpp | 118 ++++++++++++++- src/valid.cpp | 346 +++++--------------------------------------- 7 files changed, 217 insertions(+), 322 deletions(-) diff --git a/src/Att.cpp b/src/Att.cpp index 07359be70..9727fc1ee 100644 --- a/src/Att.cpp +++ b/src/Att.cpp @@ -258,8 +258,8 @@ bool Att::match (const Att& other) const // TODO after // TODO not <-- could be a problem // TODO synth - // TODO under - // TODO over + // TODO under/below + // TODO over/above // TODO first // TODO last // TODO this diff --git a/src/Cmd.cpp b/src/Cmd.cpp index 7bfe930e8..ee6a1760c 100644 --- a/src/Cmd.cpp +++ b/src/Cmd.cpp @@ -173,3 +173,50 @@ void Cmd::allCustomReports (std::vector & all) const } //////////////////////////////////////////////////////////////////////////////// +// Commands that do not directly modify the data files. +bool Cmd::isReadOnlyCommand () +{ + if (command == context.stringtable.get (CMD_ACTIVE, "active") || // R + command == context.stringtable.get (CMD_CALENDAR, "calendar") || // R + command == context.stringtable.get (CMD_COLORS, "colors") || // R + command == context.stringtable.get (CMD_COMPLETED, "completed") || // R + command == context.stringtable.get (CMD_EXPORT, "export") || // R + command == context.stringtable.get (CMD_HELP, "help") || // R + command == context.stringtable.get (CMD_HISTORY, "history") || // R + command == context.stringtable.get (CMD_GHISTORY, "ghistory") || // R + command == context.stringtable.get (CMD_INFO, "info") || // R + command == context.stringtable.get (CMD_NEXT, "next") || // R + command == context.stringtable.get (CMD_OVERDUE, "overdue") || // R + command == context.stringtable.get (CMD_PROJECTS, "projects") || // R + command == context.stringtable.get (CMD_STATS, "stats") || // R + command == context.stringtable.get (CMD_SUMMARY, "summary") || // R + command == context.stringtable.get (CMD_TAGS, "tags") || // R + command == context.stringtable.get (CMD_TIMESHEET, "timesheet") || // R + command == context.stringtable.get (CMD_VERSION, "version")) // R + return true; + + return false; +} + +//////////////////////////////////////////////////////////////////////////////// +// Commands that directly modify the data files. +bool Cmd::isWriteCommand () +{ + if (command == context.stringtable.get (CMD_ADD, "add") || // W + command == context.stringtable.get (CMD_APPEND, "append") || // W + command == context.stringtable.get (CMD_ANNOTATE, "annotate") || // W + command == context.stringtable.get (CMD_DELETE, "delete") || // W + command == context.stringtable.get (CMD_DONE, "done") || // W + command == context.stringtable.get (CMD_DUPLICATE, "duplicate") || // W + command == context.stringtable.get (CMD_EDIT, "edit") || // W + command == context.stringtable.get (CMD_IMPORT, "import") || // W + command == context.stringtable.get (CMD_START, "start") || // W + command == context.stringtable.get (CMD_STOP, "stop") || // W + command == context.stringtable.get (CMD_UNDELETE, "undelete") || // W + command == context.stringtable.get (CMD_UNDO, "undo")) // W + return true; + + return false; +} + +//////////////////////////////////////////////////////////////////////////////// diff --git a/src/Cmd.h b/src/Cmd.h index d98fb16e0..c2f9dd40a 100644 --- a/src/Cmd.h +++ b/src/Cmd.h @@ -43,6 +43,9 @@ public: void parse (const std::string&); void allCustomReports (std::vector &) const; + bool isReadOnlyCommand (); + bool isWriteCommand (); + public: std::string command; diff --git a/src/Context.cpp b/src/Context.cpp index 0c6cc3f49..c491f6a83 100644 --- a/src/Context.cpp +++ b/src/Context.cpp @@ -361,7 +361,8 @@ std::cout << "# parse sequence '" << *arg << "'" << std::endl; // Tags to include begin with '+'. else if (arg->length () > 1 && - (*arg)[0] == '+') + (*arg)[0] == '+' && + validTag (*arg)) { std::cout << "# parse tag addition '" << *arg << "'" << std::endl; if (foundSequence) @@ -372,7 +373,8 @@ std::cout << "# parse tag addition '" << *arg << "'" << std::endl; // Tags to remove begin with '-'. else if (arg->length () > 1 && - (*arg)[0] == '-') + (*arg)[0] == '-' && + validTag (*arg)) { std::cout << "# parse tag removal '" << *arg << "'" << std::endl; if (foundSequence) @@ -445,6 +447,10 @@ std::cout << "# parse post-termination description '" << *arg << "'" if (validDescription (descCandidate)) task.set ("description", descCandidate); + // TODO task.validate () + // TODO if readOnlyCommand (cmd.command) then any attributes are allowed + // TODO if writeCommand (cmd.command) then only modifiable attributes are allowed + constructFilter (); // If no command was specified, and there were no command line arguments diff --git a/src/main.h b/src/main.h index e89d781e0..cf903f64d 100644 --- a/src/main.h +++ b/src/main.h @@ -37,20 +37,21 @@ #include "../auto.h" // valid.cpp +void guess (const std::string&, const char**, std::string&); bool validPriority (const std::string&); bool validDate (std::string&); -bool validDuration (std::string&); bool validDescription (const std::string&); +bool validDuration (std::string&); void validReportColumns (const std::vector &); void validSortColumns (const std::vector &, const std::vector &); +bool isModifiableAttribute (const std::string&); +bool validAttribute (std::string&, std::string&); +bool validId (const std::string&); +bool validTag (const std::string&); // task.cpp void gatherNextTasks (/*const TDB&,*/ T&, std::vector &, std::vector &); void onChangeCallback (); -/* -std::string runTaskCommand (int, char**, TDB&, bool gc = true, bool shadow = true); -std::string runTaskCommand (std::vector &, TDB&, bool gc = false, bool shadow = false); -*/ // recur.cpp void handleRecurrence (); diff --git a/src/tests/cmd.t.cpp b/src/tests/cmd.t.cpp index 01a10f3d7..d1042afd9 100644 --- a/src/tests/cmd.t.cpp +++ b/src/tests/cmd.t.cpp @@ -33,11 +33,127 @@ Context context; //////////////////////////////////////////////////////////////////////////////// int main (int argc, char** argv) { - UnitTest t (18); + UnitTest t (76); context.config.set ("report.foo.columns", "id"); Cmd cmd; + cmd.command = "active"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand active"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand active"); + + cmd.command = "calendar"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand calendar"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand calendar"); + + cmd.command = "colors"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand colors"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand colors"); + + cmd.command = "completed"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand completed"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand completed"); + + cmd.command = "export"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand export"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand export"); + + cmd.command = "help"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand help"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand help"); + + cmd.command = "history"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand history"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand history"); + + cmd.command = "ghistory"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand ghistory"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand ghistory"); + + cmd.command = "info"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand info"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand info"); + + cmd.command = "next"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand next"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand next"); + + cmd.command = "overdue"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand overdue"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand overdue"); + + cmd.command = "projects"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand projects"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand projects"); + + cmd.command = "stats"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand stats"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand stats"); + + cmd.command = "summary"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand summary"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand summary"); + + cmd.command = "tags"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand tags"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand tags"); + + cmd.command = "timesheet"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand timesheet"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand timesheet"); + + cmd.command = "version"; + t.ok (cmd.isReadOnlyCommand (), "isReadOnlyCommand version"); + t.notok (cmd.isWriteCommand (), "not isWriteCommand version"); + + cmd.command = "add"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand add"); + t.ok (cmd.isWriteCommand (), "isWriteCommand add"); + + cmd.command = "append"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand append"); + t.ok (cmd.isWriteCommand (), "isWriteCommand append"); + + cmd.command = "annotate"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand annotate"); + t.ok (cmd.isWriteCommand (), "isWriteCommand annotate"); + + cmd.command = "delete"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand delete"); + t.ok (cmd.isWriteCommand (), "isWriteCommand delete"); + + cmd.command = "done"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand done"); + t.ok (cmd.isWriteCommand (), "isWriteCommand done"); + + cmd.command = "duplicate"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand duplicate"); + t.ok (cmd.isWriteCommand (), "isWriteCommand duplicate"); + + cmd.command = "edit"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand edit"); + t.ok (cmd.isWriteCommand (), "isWriteCommand edit"); + + cmd.command = "import"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand import"); + t.ok (cmd.isWriteCommand (), "isWriteCommand import"); + + cmd.command = "start"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand start"); + t.ok (cmd.isWriteCommand (), "isWriteCommand start"); + + cmd.command = "stop"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand stop"); + t.ok (cmd.isWriteCommand (), "isWriteCommand stop"); + + cmd.command = "undelete"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand undelete"); + t.ok (cmd.isWriteCommand (), "isWriteCommand undelete"); + + cmd.command = "undo"; + t.notok (cmd.isReadOnlyCommand (), "not isReadOnlyCommand undo"); + t.ok (cmd.isWriteCommand (), "isWriteCommand undo"); + t.ok (cmd.valid ("annotate"), "Cmd::valid annotate"); t.ok (cmd.valid ("annotat"), "Cmd::valid annotat"); t.ok (cmd.valid ("annota"), "Cmd::valid annota"); diff --git a/src/valid.cpp b/src/valid.cpp index 86d1f9c2e..69b2addea 100644 --- a/src/valid.cpp +++ b/src/valid.cpp @@ -42,6 +42,7 @@ extern Context context; //////////////////////////////////////////////////////////////////////////////// // NOTE: These are static arrays only because there is no initializer list for // std::vector. +// TODO Obsolete static const char* colors[] = { "bold", @@ -104,6 +105,7 @@ static const char* colors[] = "", }; +// TODO Obsolete static const char* attributes[] = { "project", @@ -118,44 +120,23 @@ static const char* attributes[] = "until", "mask", "imask", +// "limit", "", }; -// Alphabetical please. -static const char* commands[] = +static const char* modifiableAttributes[] = { - "active", - "add", - "append", - "annotate", - "calendar", - "colors", - "completed", - "delete", - "done", - "duplicate", - "edit", - "export", - "help", - "history", - "ghistory", - "import", - "info", - "next", - "overdue", - "projects", - "start", - "stats", - "stop", - "summary", - "tags", - "timesheet", - "undelete", - "undo", - "version", + "project", + "priority", + "fg", + "bg", + "due", + "recur", + "until", "", }; +// TODO Relocate inside Context. static std::vector customReports; //////////////////////////////////////////////////////////////////////////////// @@ -171,25 +152,6 @@ void guess ( guess (type, options, candidate); } -//////////////////////////////////////////////////////////////////////////////// -static bool isCommand (const std::string& candidate) -{ - std::vector options; - for (int i = 0; commands[i][0]; ++i) - options.push_back (commands[i]); - - std::vector matches; - autoComplete (candidate, options, matches); - if (0 == matches.size ()) - { - autoComplete (candidate, customReports, matches); - if (0 == matches.size ()) - return false; - } - - return true; -} - //////////////////////////////////////////////////////////////////////////////// bool validDate (std::string& date) { @@ -217,9 +179,26 @@ bool validPriority (const std::string& input) } //////////////////////////////////////////////////////////////////////////////// -static bool validAttribute ( - std::string& name, - std::string& value) +// Only attributes that are written to the data files. +// TODO Relocate to Att.cpp. +bool isModifiableAttribute (const std::string& name) +{ + if (name == "project" || + name == "priority" || + name == "fg" || + name == "bg" || + name == "due" || + name == "recur" || + name == "until") + return true; + + return false; +} + +//////////////////////////////////////////////////////////////////////////////// +// All attributes, regardless of usage. +// TODO Relocate to Att.cpp. +bool validAttribute (std::string& name, std::string& value) { guess ("attribute", attributes, name); if (name != "") @@ -256,7 +235,7 @@ static bool validAttribute ( } //////////////////////////////////////////////////////////////////////////////// -static bool validId (const std::string& input) +bool validId (const std::string& input) { if (input.length () == 0) return false; @@ -269,59 +248,7 @@ static bool validId (const std::string& input) } //////////////////////////////////////////////////////////////////////////////// -// 1,2-4,6 -static bool validSequence ( - const std::string& input, - std::vector & ids) -{ - std::vector ranges; - split (ranges, input, ','); - - std::vector ::iterator it; - for (it = ranges.begin (); it != ranges.end (); ++it) - { - std::vector range; - split (range, *it, '-'); - - switch (range.size ()) - { - case 1: - { - if (! validId (range[0])) - return false; - - int id = ::atoi (range[0].c_str ()); - ids.push_back (id); - } - break; - - case 2: - { - if (! validId (range[0]) || - ! validId (range[1])) - return false; - - int low = ::atoi (range[0].c_str ()); - int high = ::atoi (range[1].c_str ()); - if (low >= high) - return false; - - for (int i = low; i <= high; ++i) - ids.push_back (i); - } - break; - - default: - return false; - break; - } - } - - return ids.size () ? true : false; -} - -//////////////////////////////////////////////////////////////////////////////// -static bool validTag (const std::string& input) +bool validTag (const std::string& input) { if ((input[0] == '-' || input[0] == '+') && input.length () > 1) @@ -342,62 +269,6 @@ bool validDescription (const std::string& input) return false; } -//////////////////////////////////////////////////////////////////////////////// -bool validCommand (std::string& input) -{ - std::string copy = input; - guess ("command", commands, copy); - if (copy == "") - { - copy = input; - guess ("command", customReports, copy); - if (copy == "") - return false; - } - - input = copy; - return true; -} - -//////////////////////////////////////////////////////////////////////////////// -static bool validSubstitution ( - std::string& input, - std::string& from, - std::string& to, - bool& global) -{ - size_t first = input.find ('/'); - if (first != std::string::npos) - { - size_t second = input.find ('/', first + 1); - if (second != std::string::npos) - { - size_t third = input.find ('/', second + 1); - if (third != std::string::npos) - { - if (first == 0 && - first < second && - second < third && - (third == input.length () - 1 || - third == input.length () - 2)) - { - from = input.substr (first + 1, second - first - 1); - to = input.substr (second + 1, third - second - 1); - - global = false; - if (third == input.length () - 2 && - input.find ('g', third + 1) != std::string::npos) - global = true; - - return true; - } - } - } - } - - return false; -} - //////////////////////////////////////////////////////////////////////////////// bool validDuration (std::string& input) { @@ -406,155 +277,6 @@ bool validDuration (std::string& input) return true; } -//////////////////////////////////////////////////////////////////////////////// -// Token EBNF -// ------- ---------------------------------- -// command first non-id recognized argument -// -// substitution ::= "/" from "/" to "/g" -// | "/" from "/" to "/" ; -// -// tags ::= "+" word -// | "-" word ; -// -// attributes ::= word ":" value -// | word ":" -// -// sequence ::= \d+ "," sequence -// | \d+ "-" \d+ ; -// -// description (whatever isn't one of the above) -/* -void parse ( - std::vector & args, - std::string& command, - T& task) -{ - command = ""; - - bool terminated = false; - bool foundSequence = false; - bool foundSomethingAfterSequence = false; - - std::string descCandidate = ""; - for (size_t i = 0; i < args.size (); ++i) - { - std::string arg (args[i]); - - if (!terminated) - { - size_t colon; // Pointer to colon in argument. - std::string from; - std::string to; - bool global; - std::vector sequence; - - // The '--' argument shuts off all parsing - everything is an argument. - if (arg == "--") - terminated = true; - - // An id is the first argument found that contains all digits. - else if (lowerCase (command) != "add" && // "add" doesn't require an ID - validSequence (arg, sequence) && - ! foundSomethingAfterSequence) - { - foundSequence = true; - foreach (id, sequence) - task.addId (*id); - } - - // 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] == '-') - task.addRemoveTag (arg.substr (1, std::string::npos)); - } - - // Attributes contain a constant string followed by a colon, followed by a - // 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); - - if (validAttribute (name, value)) - { - if (name != "recur" || validDuration (value)) - task.setAttribute (name, value); - } - - // If it is not a valid attribute, then allow the argument as part of - // the description. - else - { - if (descCandidate.length ()) - descCandidate += " "; - descCandidate += arg; - } - } - - // 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; - else - { - if (descCandidate.length ()) - descCandidate += " "; - descCandidate += arg; - } - } - - // Anything else is just considered description. - else - { - if (foundSequence) - foundSomethingAfterSequence = true; - - if (descCandidate.length ()) - descCandidate += " "; - descCandidate += arg; - } - } - // terminated, therefore everything subsequently is a description. - else - { - if (foundSequence) - foundSomethingAfterSequence = true; - - if (descCandidate.length ()) - descCandidate += " "; - descCandidate += arg; - } - } - - if (validDescription (descCandidate)) - task.setDescription (descCandidate); -} -*/ - //////////////////////////////////////////////////////////////////////////////// void validReportColumns (const std::vector & columns) {