From 77200d7eb4049f75b3c9d8f3bb8d7d27c4172421 Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Sun, 4 Mar 2012 00:05:11 -0500 Subject: [PATCH] Bug #837 - Fixed bug #837, which caused incorrect urgency calculations for tasks that have completed dependencies, and problems when editing those tasks (thanks to Matt Kraai). - Added Task::addDependency (const std::string&) method for reconstructing dependencies on tasks with no ID. - Modfified Task::urgency_blocked so that it considers the blocking task's status. This is an expensive test, and so it is wrapped by a cheaper test to see if there are/were any dependencies at all. This means that urgency calculations are not slowed for tasks without dependencies, and is slower but more accurate for tasks that do have dependencies. - Modified edit command so that dependencies are shown as IDs or UUIDs depending on whether the task is pending or not. - Modified edit command so that dependencies are parsed as IDs or UUIDs depending on whether the task is pending or not. - Modified wording in the template of the edit command to reflect the above. - Added unit tests bug.837.t. --- ChangeLog | 3 ++ src/Task.cpp | 43 ++++++++++++++++++++++++- src/Task.h | 1 + src/commands/CmdEdit.cpp | 31 +++++++++++++++--- src/en-US.h | 2 +- test/bug.837.t | 69 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 142 insertions(+), 7 deletions(-) create mode 100755 test/bug.837.t diff --git a/ChangeLog b/ChangeLog index 2122abb10..674fa5252 100644 --- a/ChangeLog +++ b/ChangeLog @@ -231,6 +231,9 @@ + Fixed bug #835, which prevented hierarchical projects from being recognized. + Fixed bug #836, which preserves numeric arguments as-is (thanks to Matt Kraai for the patch). + + Fixed bug #837, which caused incorrect urgency calculations for tasks that + have completed dependencies, and problems when editing those tasks (thanks + to Matt Kraai). + Fixed bug #839, which caused problems when recurrence frequencies of '1m' were used. This is an obsolete form, and should now be '1mo' (thanks to Gour D). diff --git a/src/Task.cpp b/src/Task.cpp index 6c18cc971..f7fccf333 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -743,6 +743,32 @@ void Task::addDependency (int id) recalc_urgency = true; } +//////////////////////////////////////////////////////////////////////////////// +void Task::addDependency (const std::string& uuid) +{ + if (uuid == get ("uuid")) + throw std::string (STRING_TASK_DEPEND_ITSELF); + + // 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->get ("uuid"), uuid); + } + else + set ("depends", uuid); + + // Prevent circular dependencies. + if (dependencyIsCircular (*this)) + throw std::string (STRING_TASK_DEPEND_CIRCULAR); + + recalc_urgency = true; +} + //////////////////////////////////////////////////////////////////////////////// void Task::removeDependency (const std::string& uuid) { @@ -1333,10 +1359,25 @@ float Task::urgency_waiting () const } //////////////////////////////////////////////////////////////////////////////// +// A task is blocked only if the task it depends upon is pending/waiting. float Task::urgency_blocked () const { if (has ("depends")) - return 1.0; + { + std::vector deps; + getDependencies (deps); + + std::vector ::iterator d; + for (d = deps.begin (); d != deps.end (); ++d) + { + Task t; + if (context.tdb2.get (*d, t) && + (t.getStatus () == Task::pending || t.getStatus () == Task::waiting)) + { + return 1.0; + } + } + } return 0.0; } diff --git a/src/Task.h b/src/Task.h index 46e5a5ee8..12997fd8b 100644 --- a/src/Task.h +++ b/src/Task.h @@ -91,6 +91,7 @@ public: void removeAnnotations (); void addDependency (int); + void addDependency (const std::string&); void removeDependency (int); void removeDependency (const std::string&); void getDependencies (std::vector &) const; diff --git a/src/commands/CmdEdit.cpp b/src/commands/CmdEdit.cpp index 79992e35a..cba3999c9 100644 --- a/src/commands/CmdEdit.cpp +++ b/src/commands/CmdEdit.cpp @@ -212,15 +212,27 @@ std::string CmdEdit::formatTask (Task task) before << " Annotation: " << now.toString (context.config.get ("dateformat.annotation")) << " -- \n"; // Add dependencies here. - std::vector dependencies; + std::vector dependencies; task.getDependencies (dependencies); - std::string allDeps; - join (allDeps, ",", dependencies); + std::stringstream allDeps; + for (unsigned int i = 0; i < dependencies.size (); ++i) + { + if (i) + allDeps << ","; + + Task t; + context.tdb2.get (dependencies[i], t); + if (t.getStatus () == Task::pending || + t.getStatus () == Task::waiting) + allDeps << t.id; + else + allDeps << dependencies[i]; + } if (verbose) before << "# " << STRING_EDIT_DEP_SEP << "\n"; - before << " Dependencies: " << allDeps << "\n"; + before << " Dependencies: " << allDeps.str () << "\n"; before << "# " << STRING_EDIT_END << "\n"; return before.str (); @@ -582,9 +594,18 @@ void CmdEdit::parseTask (Task& task, const std::string& after) std::vector ::iterator dep; for (dep = dependencies.begin (); dep != dependencies.end (); ++dep) { - int id = atoi (dep->c_str ()); + int id = 0; + + // Crude UUID check. + if (dep->length () == 36) + id = context.tdb2.pending.id (*dep); + else + id = atoi (dep->c_str ()); + if (id) task.addDependency (id); + else + task.addDependency (*dep); } } diff --git a/src/en-US.h b/src/en-US.h index 31f89e2d0..42657132a 100644 --- a/src/en-US.h +++ b/src/en-US.h @@ -597,7 +597,7 @@ #define STRING_EDIT_UNPARSEABLE "Taskwarrior couldn't handle your edits. Would you like to try again?" #define STRING_EDIT_UNWRITABLE "Your data.location directory is not writable." #define STRING_EDIT_TAG_SEP "Separate the tags with spaces, like this: tag1 tag2" -#define STRING_EDIT_DEP_SEP "Dependencies should be a comma-separated list of task IDs, with no spaces." +#define STRING_EDIT_DEP_SEP "Dependencies should be a comma-separated list of task IDs/UUIDs, with no spaces." #define STRING_EDIT_END "End" #define STRING_EDIT_PROJECT_MOD "Project modified." diff --git a/test/bug.837.t b/test/bug.837.t new file mode 100755 index 000000000..d71b373d7 --- /dev/null +++ b/test/bug.837.t @@ -0,0 +1,69 @@ +#! /usr/bin/perl +################################################################################ +## taskwarrior - a command line task list manager. +## +## Copyright 2006-2012, Paul Beckingham, Federico Hernandez. +## +## Permission is hereby granted, free of charge, to any person obtaining a copy +## of this software and associated documentation files (the "Software"), to deal +## in the Software without restriction, including without limitation the rights +## to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +## copies of the Software, and to permit persons to whom the Software is +## furnished to do so, subject to the following conditions: +## +## The above copyright notice and this permission notice shall be included +## in all copies or substantial portions of the Software. +## +## THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +## OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +## FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +## THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +## LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +## OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +## SOFTWARE. +## +## http://www.opensource.org/licenses/mit-license.php +## +################################################################################ + +use strict; +use warnings; +use Test::More tests => 5; + +# 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 837: When a task is completed, tasks that depended upon it don't have +# correct urgency and depend on 0 when edited +qx{../src/task rc:bug.rc add one}; +qx{../src/task rc:bug.rc add two dep:1}; +my $output = qx{../src/task rc:bug.rc long}; + +$output = qx{../src/task rc:bug.rc 1 _urgency}; +like ($output, qr/ 8\n/, 'blocking urgency == 8'); + +$output = qx{../src/task rc:bug.rc 2 _urgency}; +like ($output, qr/ -5\n/, 'blocked urgency == -5'); + +qx{../src/task rc:bug.rc 1 done}; +qx{../src/task rc:bug.rc list}; + +$output = qx{../src/task rc:bug.rc 1 _urgency}; +like ($output, qr/ 0\n/, 'unblocked urgency == 0'); + +# Cleanup. +unlink qw(pending.data completed.data undo.data backlog.data synch.key bug.rc); +ok (! -r 'pending.data' && + ! -r 'completed.data' && + ! -r 'undo.data' && + ! -r 'backlog.data' && + ! -r 'synch.key' && + ! -r 'bug.rc', 'Cleanup'); + +exit 0; +