From 1364202d305a68aebc874e79935ac77e69a314c5 Mon Sep 17 00:00:00 2001 From: Louis-Claude Canon Date: Sat, 5 May 2012 10:22:05 +0200 Subject: [PATCH] Feature - Allow UUIDs and IDs range when modifying task dependencies. - Update man page. - Add unit tests. - Fatorize code when adding and removing dependencies in Task.cpp. --- doc/man/task.1.in | 4 +++- src/Task.cpp | 29 +++++++---------------------- src/commands/Command.cpp | 32 ++++++++++++++++++++++++++------ src/en-US.h | 4 ++-- test/dependencies.t | 35 ++++++++++++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 32 deletions(-) diff --git a/doc/man/task.1.in b/doc/man/task.1.in index ad29a202c..d5fbd996a 100644 --- a/doc/man/task.1.in +++ b/doc/man/task.1.in @@ -566,7 +566,9 @@ Date until task becomes pending. .B depends: Declares this task to be dependent on id1 and id2. This means that the tasks id1 and id2 should be completed before this task. Consequently, this task will -then show up on the 'blocked' report. +then show up on the 'blocked' report. It accepts a comma-separated list of ID +numbers, UUID numbers and ID ranges. When prefixing any element of this list +by '-', the specified tasks are removed from the dependency list. .TP .B entry: diff --git a/src/Task.cpp b/src/Task.cpp index 9e1309a65..73dd99b22 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -767,32 +767,16 @@ void Task::removeAnnotations () //////////////////////////////////////////////////////////////////////////////// void Task::addDependency (int id) { - if (id == this->id) - throw std::string (STRING_TASK_DEPEND_ITSELF); - // Check that id is resolvable. std::string uuid = context.tdb2.pending.uuid (id); if (uuid == "") - throw format (STRING_TASK_DEPEND_MISSING, id); + throw format (STRING_TASK_DEPEND_MISS_CREA, id); - // Store the dependency. std::string depends = get ("depends"); - if (depends != "") - { - // Check for extant dependency. - if (depends.find (uuid) == std::string::npos) - set ("depends", depends + "," + uuid); - else - throw format (STRING_TASK_DEPEND_DUP, this->id, id); - } - else - set ("depends", uuid); + if (depends.find (uuid) != std::string::npos) + throw format (STRING_TASK_DEPEND_DUP, this->id, id); - // Prevent circular dependencies. - if (dependencyIsCircular (*this)) - throw std::string (STRING_TASK_DEPEND_CIRCULAR); - - recalc_urgency = true; + addDependency(uuid); } //////////////////////////////////////////////////////////////////////////////// @@ -849,11 +833,12 @@ void Task::removeDependency (const std::string& uuid) //////////////////////////////////////////////////////////////////////////////// void Task::removeDependency (int id) { + std::string depends = get ("depends"); std::string uuid = context.tdb2.pending.uuid (id); - if (uuid != "") + if (uuid != "" && depends.find (uuid) != std::string::npos) removeDependency (uuid); else - throw std::string (STRING_TASK_DEPEND_NO_UUID); + throw format (STRING_TASK_DEPEND_MISS_DEL, id); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/commands/Command.cpp b/src/commands/Command.cpp index cf50a7648..4e5f73c08 100644 --- a/src/commands/Command.cpp +++ b/src/commands/Command.cpp @@ -478,10 +478,10 @@ void Command::modify_task ( } else { - // Dependencies must be resolved to UUIDs. + // Dependencies are used as IDs. if (name == "depends") { - // Convert ID to UUID. + // Parse IDs std::vector deps; split (deps, value, ','); @@ -489,11 +489,31 @@ void Command::modify_task ( std::vector ::iterator i; for (i = deps.begin (); i != deps.end (); i++) { - int id = strtol (i->c_str (), NULL, 10); - if (id < 0) - task.removeDependency (-id); + bool removal = false; + std::string& dep = *i; + + if (dep[0] == '-') + { + removal = true; + dep = i->substr(1, std::string::npos); + } + + std::vector ids; + // Crude UUID check + if (dep.length () == 36) + { + int id = context.tdb2.pending.id (dep); + ids.push_back (id); + } else - task.addDependency (id); + A3::extract_id (dep, ids); + + std::vector ::iterator id; + for (id = ids.begin (); id != ids.end(); id++) + if (removal) + task.removeDependency (*id); + else + task.addDependency (*id); } } diff --git a/src/en-US.h b/src/en-US.h index 2242b752f..23286d76c 100644 --- a/src/en-US.h +++ b/src/en-US.h @@ -755,10 +755,10 @@ #define STRING_TASK_PARSE_TOO_SHORT "Line too short." #define STRING_TASK_PARSE_UNREC_FF "Unrecognized taskwarrior file format." #define STRING_TASK_DEPEND_ITSELF "A task cannot be dependent on itself." -#define STRING_TASK_DEPEND_MISSING "Could not create a dependency on task {1} - not found." +#define STRING_TASK_DEPEND_MISS_CREA "Could not create a dependency on task {1} - not found." +#define STRING_TASK_DEPEND_MISS_DEL "Could not delete a dependency on task {1} - not found." #define STRING_TASK_DEPEND_DUP "Task {1} already depends on task {2}." #define STRING_TASK_DEPEND_CIRCULAR "Circular dependency detected and disallowed." -#define STRING_TASK_DEPEND_NO_UUID "Could not find a UUID for id {1}." #define STRING_TASK_VALID_DESC "A task must have a description." #define STRING_TASK_VALID_BLANK "Cannot add a task that is blank." #define STRING_TASK_VALID_WAIT "Warning: You have specified a 'wait' date that is after the 'due' date." diff --git a/test/dependencies.t b/test/dependencies.t index de4aa9b8f..22519b36e 100755 --- a/test/dependencies.t +++ b/test/dependencies.t @@ -28,7 +28,7 @@ use strict; use warnings; -use Test::More tests => 38; +use Test::More tests => 49; # Create the rc file. if (open my $fh, '>', 'dep.rc') @@ -217,6 +217,39 @@ qx{../src/task rc:dep.rc 2 modify dep:-1}; $output = qx{../src/task rc:dep.rc depreport}; like ($output, qr/\s1\s+One\s*\n\s2\s+Four\s*\n\s3\s+2\s+Five/, 'dependencies - chain should not be automatically repaired after manually removing a dependency'); +# [38] +unlink 'pending.data'; +ok (!-r 'pending.data', 'Removed pending.data for a fresh start'); + +# Bug when adding a range of dependencies - 'task 3 mod dep:1-2' interprets the +# range 1-2 as the id 1 + +qx{../src/task rc:dep.rc add test1}; +qx{../src/task rc:dep.rc add test2}; +qx{../src/task rc:dep.rc add test3}; +qx{../src/task rc:dep.rc add test4}; +qx{../src/task rc:dep.rc add test5}; +my $output = qx{../src/task rc:dep.rc 5 info}; +my ($uuid) = $output =~ /UUID\s+(\S+)/; + +# [39-43] test a comma-separated list of IDs, UUIDs, and ID ranges for creation +qx{../src/task rc:dep.rc add test6 dep:1-3,4,$uuid}; +$output = qx{../src/task rc:dep.rc 6 info}; +like ($output, qr/test1/ms, 'Dependency appearing for task1'); +like ($output, qr/test2/ms, 'Dependency appearing for task2'); +like ($output, qr/test3/ms, 'Dependency appearing for task3'); +like ($output, qr/test4/ms, 'Dependency appearing for task4'); +like ($output, qr/test5/ms, 'Dependency appearing for task5'); + +# [44-48] test a comma-separated list of IDs, UUIDs, and ID ranges for deletion +qx{../src/task rc:dep.rc 6 mod dep:-1-3,-4,-$uuid}; +$output = qx{../src/task rc:dep.rc 6 info}; +unlike ($output, qr/test1/ms, 'Dependency not appearing for task1'); +unlike ($output, qr/test2/ms, 'Dependency not appearing for task2'); +unlike ($output, qr/test3/ms, 'Dependency not appearing for task3'); +unlike ($output, qr/test4/ms, 'Dependency not appearing for task4'); +unlike ($output, qr/test5/ms, 'Dependency not appearing for task5'); + # TODO - test dependency.confirmation config variable # TODO - test undo on backing out chain gap repair # TODO - test undo on backing out choice to not perform chain gap repair