diff --git a/src/TDB2.cpp b/src/TDB2.cpp index a5f8646ba..49313a3b3 100644 --- a/src/TDB2.cpp +++ b/src/TDB2.cpp @@ -516,29 +516,26 @@ void TF2::dependency_scan () // Iterate and modify TDB2 in-place. Don't do this at home. for (auto& left : _tasks) { - if (left.has ("depends")) + for (auto& dep : left.getDependencyUUIDs ()) { - for (auto& dep : left.getDependencyUUIDs ()) + for (auto& right : _tasks) { - for (auto& right : _tasks) + if (right.get ("uuid") == dep) { - if (right.get ("uuid") == dep) + // GC hasn't run yet, check both tasks for their current status + Task::status lstatus = left.getStatus (); + Task::status rstatus = right.getStatus (); + if (lstatus != Task::completed && + lstatus != Task::deleted && + rstatus != Task::completed && + rstatus != Task::deleted) { - // GC hasn't run yet, check both tasks for their current status - Task::status lstatus = left.getStatus (); - Task::status rstatus = right.getStatus (); - if (lstatus != Task::completed && - lstatus != Task::deleted && - rstatus != Task::completed && - rstatus != Task::deleted) - { - left.is_blocked = true; - right.is_blocking = true; - } - - // Only want to break out of the "right" loop. - break; + left.is_blocked = true; + right.is_blocking = true; } + + // Only want to break out of the "right" loop. + break; } } } diff --git a/src/Task.cpp b/src/Task.cpp index 50b8e8bcd..081f4b373 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -1258,6 +1258,15 @@ void Task::removeDependency (int id) throw format ("Could not delete a dependency on task {1} - not found.", id); } +//////////////////////////////////////////////////////////////////////////////// +bool Task::hasDependency (const std::string& uuid) const +{ + auto deps = split (get ("depends"), ','); + + auto i = std::find (deps.begin (), deps.end (), uuid); + return i != deps.end (); +} + //////////////////////////////////////////////////////////////////////////////// std::vector Task::getDependencyIDs () const { @@ -1277,15 +1286,36 @@ std::vector Task::getDependencyUUIDs () const //////////////////////////////////////////////////////////////////////////////// std::vector Task::getDependencyTasks () const { - std::vector all; - for (auto& dep : split (get ("depends"), ',')) - { - Task task; - Context::getContext ().tdb2.get (dep, task); - all.push_back (task); - } + auto depends = get ("depends"); - return all; + // NOTE: this may seem inefficient, but note that `TDB2::get` performs a + // linear search on each invocation, so scanning *once* is quite a bit more + // efficient. + std::vector blocking; + if (depends != "") + for (auto& it : Context::getContext ().tdb2.pending.get_tasks ()) + if (it.getStatus () != Task::completed && + it.getStatus () != Task::deleted && + depends.find (it.get ("uuid")) != std::string::npos) + blocking.push_back (it); + + return blocking; +} + +//////////////////////////////////////////////////////////////////////////////// +std::vector Task::getBlockedTasks () const +{ + auto uuid = get ("uuid"); + + std::vector blocked; + for (auto& it : Context::getContext ().tdb2.pending.get_tasks ()) + if (it.getStatus () != Task::completed && + it.getStatus () != Task::deleted && + it.has ("depends") && + it.get ("depends").find (uuid) != std::string::npos) + blocked.push_back (it); + + return blocked; } #endif @@ -2038,9 +2068,9 @@ float Task::urgency_inherit () const { float v = FLT_MIN; #ifdef PRODUCT_TASKWARRIOR - // Calling dependencyGetBlocked is rather expensive. + // Calling getBlockedTasks is rather expensive. // It is called recursively for each dependency in the chain here. - for (auto& task : dependencyGetBlocked (*this)) + for (auto& task : getBlockedTasks ()) { // Find highest urgency in all blocked tasks. v = std::max (v, task.urgency ()); diff --git a/src/Task.h b/src/Task.h index 1936e2a50..5dbc679d5 100644 --- a/src/Task.h +++ b/src/Task.h @@ -144,8 +144,10 @@ public: #ifdef PRODUCT_TASKWARRIOR void removeDependency (int); void removeDependency (const std::string&); + bool hasDependency (const std::string&) const; std::vector getDependencyIDs () const; std::vector getDependencyUUIDs () const; + std::vector getBlockedTasks () const; std::vector getDependencyTasks () const; std::vector getUDAOrphanUUIDs () const; diff --git a/src/columns/ColDepends.cpp b/src/columns/ColDepends.cpp index ba66547c9..c430e051c 100644 --- a/src/columns/ColDepends.cpp +++ b/src/columns/ColDepends.cpp @@ -68,7 +68,9 @@ void ColumnDepends::setStyle (const std::string& value) void ColumnDepends::measure (Task& task, unsigned int& minimum, unsigned int& maximum) { minimum = maximum = 0; - if (task.has (_name)) + auto deptasks = task.getDependencyTasks (); + + if (deptasks.size () > 0) { if (_style == "indicator") { @@ -77,25 +79,24 @@ void ColumnDepends::measure (Task& task, unsigned int& minimum, unsigned int& ma else if (_style == "count") { - minimum = maximum = 2 + format ((int) dependencyGetBlocking (task).size ()).length (); + minimum = maximum = 2 + format ((int) deptasks.size ()).length (); } else if (_style == "default" || _style == "list") { minimum = maximum = 0; - auto blocking = dependencyGetBlocking (task); std::vector blocking_ids; - blocking_ids.reserve(blocking.size()); - for (auto& i : blocking) + blocking_ids.reserve(deptasks.size()); + for (auto& i : deptasks) blocking_ids.push_back (i.id); auto all = join (" ", blocking_ids); maximum = all.length (); unsigned int length; - for (auto& i : blocking) + for (auto& i : deptasks) { length = format (i.id).length (); if (length > minimum) @@ -112,7 +113,9 @@ void ColumnDepends::render ( int width, Color& color) { - if (task.has (_name)) + auto deptasks = task.getDependencyTasks (); + + if (deptasks.size () > 0) { if (_style == "indicator") { @@ -121,17 +124,15 @@ void ColumnDepends::render ( else if (_style == "count") { - renderStringRight (lines, width, color, '[' + format (static_cast (dependencyGetBlocking (task).size ())) + ']'); + renderStringRight (lines, width, color, '[' + format (static_cast (deptasks.size ())) + ']'); } else if (_style == "default" || _style == "list") { - auto blocking = dependencyGetBlocking (task); - std::vector blocking_ids; - blocking_ids.reserve(blocking.size()); - for (const auto& t : blocking) + blocking_ids.reserve(deptasks.size()); + for (const auto& t : deptasks) blocking_ids.push_back (t.id); auto combined = join (" ", blocking_ids); diff --git a/src/commands/CmdEdit.cpp b/src/commands/CmdEdit.cpp index 0c69e4faa..325d3b433 100644 --- a/src/commands/CmdEdit.cpp +++ b/src/commands/CmdEdit.cpp @@ -667,7 +667,8 @@ void CmdEdit::parseTask (Task& task, const std::string& after, const std::string value = findValue (after, "\n Dependencies:"); auto dependencies = split (value, ','); - task.remove ("depends"); + for (auto& dep : task.getDependencyUUIDs ()) + task.removeDependency (dep); for (auto& dep : dependencies) { if (dep.length () >= 7) diff --git a/src/commands/CmdInfo.cpp b/src/commands/CmdInfo.cpp index 0d719a2d7..25115aaf4 100644 --- a/src/commands/CmdInfo.cpp +++ b/src/commands/CmdInfo.cpp @@ -147,7 +147,7 @@ int CmdInfo::execute (std::string& output) // dependencies: blocked { - auto blocked = dependencyGetBlocking (task); + auto blocked = task.getDependencyTasks (); if (blocked.size ()) { std::stringstream message; @@ -162,7 +162,7 @@ int CmdInfo::execute (std::string& output) // dependencies: blocking { - auto blocking = dependencyGetBlocked (task); + auto blocking = task.getBlockedTasks (); if (blocking.size ()) { std::stringstream message; diff --git a/src/commands/CmdPurge.cpp b/src/commands/CmdPurge.cpp index 44eaefd09..0c94906fc 100644 --- a/src/commands/CmdPurge.cpp +++ b/src/commands/CmdPurge.cpp @@ -71,8 +71,7 @@ void CmdPurge::handleDeps (Task& task) for (auto& blockedConst: Context::getContext ().tdb2.all_tasks ()) { Task& blocked = const_cast(blockedConst); - if (blocked.has ("depends") && - blocked.get ("depends").find (uuid) != std::string::npos) + if (blocked.hasDependency (uuid)) { blocked.removeDependency (uuid); Context::getContext ().tdb2.modify (blocked); diff --git a/src/dependency.cpp b/src/dependency.cpp index d05e4a450..f422971c9 100644 --- a/src/dependency.cpp +++ b/src/dependency.cpp @@ -36,38 +36,6 @@ #define STRING_DEPEND_BLOCKED "Task {1} is blocked by:" -//////////////////////////////////////////////////////////////////////////////// -std::vector dependencyGetBlocked (const Task& task) -{ - auto uuid = task.get ("uuid"); - - std::vector blocked; - for (auto& it : Context::getContext ().tdb2.pending.get_tasks ()) - if (it.getStatus () != Task::completed && - it.getStatus () != Task::deleted && - it.has ("depends") && - it.get ("depends").find (uuid) != std::string::npos) - blocked.push_back (it); - - return blocked; -} - -//////////////////////////////////////////////////////////////////////////////// -std::vector dependencyGetBlocking (const Task& task) -{ - auto depends = task.get ("depends"); - - std::vector blocking; - if (depends != "") - for (auto& it : Context::getContext ().tdb2.pending.get_tasks ()) - if (it.getStatus () != Task::completed && - it.getStatus () != Task::deleted && - depends.find (it.get ("uuid")) != std::string::npos) - blocking.push_back (it); - - return blocking; -} - //////////////////////////////////////////////////////////////////////////////// // Returns true if the supplied task adds a cycle to the dependency chain. bool dependencyIsCircular (const Task& task) @@ -150,12 +118,12 @@ bool dependencyIsCircular (const Task& task) // void dependencyChainOnComplete (Task& task) { - auto blocking = dependencyGetBlocking (task); + auto blocking = task.getDependencyTasks (); // If the task is anything but the tail end of a dependency chain. if (blocking.size ()) { - auto blocked = dependencyGetBlocked (task); + auto blocked = task.getBlockedTasks (); // Nag about broken chain. if (Context::getContext ().config.getBoolean ("dependency.reminder")) @@ -207,7 +175,7 @@ void dependencyChainOnStart (Task& task) { if (Context::getContext ().config.getBoolean ("dependency.reminder")) { - auto blocking = dependencyGetBlocking (task); + auto blocking = task.getDependencyTasks (); // If the task is anything but the tail end of a dependency chain, nag about // broken chain. diff --git a/src/feedback.cpp b/src/feedback.cpp index a2133a9a8..35fc83baf 100644 --- a/src/feedback.cpp +++ b/src/feedback.cpp @@ -78,6 +78,7 @@ std::string taskDifferences (const Task& before, const Task& after) << format ("{1} will be deleted.", Lexer::ucFirst (name)) << "\n"; + // TODO: #2572 - rewrite to look at dep_ and tag_ for (auto& name : afterOnly) { if (name == "depends") @@ -384,12 +385,12 @@ void feedback_unblocked (const Task& task) if (Context::getContext ().verbose ("affected")) { // Get a list of tasks that depended on this task. - auto blocked = dependencyGetBlocked (task); + auto blocked = task.getBlockedTasks (); // Scan all the tasks that were blocked by this task for (auto& i : blocked) { - auto blocking = dependencyGetBlocking (i); + auto blocking = i.getDependencyTasks (); if (blocking.size () == 0) { if (i.id) diff --git a/src/main.h b/src/main.h index 324b4c2ed..e501300b9 100644 --- a/src/main.h +++ b/src/main.h @@ -59,8 +59,6 @@ std::string colorizeError (const std::string&); std::string colorizeDebug (const std::string&); // dependency.cpp -std::vector dependencyGetBlocked (const Task&); -std::vector dependencyGetBlocking (const Task&); bool dependencyIsCircular (const Task&); void dependencyChainOnComplete (Task&); void dependencyChainOnStart (Task&); diff --git a/src/sort.cpp b/src/sort.cpp index e1421b580..92056c366 100644 --- a/src/sort.cpp +++ b/src/sort.cpp @@ -200,22 +200,25 @@ static bool sort_compare (int left, int right) // Depends string. else if (field == "depends") { - // Raw data is a comma-separated list of uuids - auto left_string = (*global_data)[left].get_ref (field); - auto right_string = (*global_data)[right].get_ref (field); + // Raw data is an un-sorted list of UUIDs. We just need a stable + // sort, so we sort them lexically. + auto left_deps = (*global_data)[left].getDependencyUUIDs (); + std::sort(left_deps.begin(), left_deps.end()); + auto right_deps = (*global_data)[right].getDependencyUUIDs (); + std::sort(right_deps.begin(), right_deps.end()); - if (left_string == right_string) + if (left_deps == right_deps) continue; - if (left_string == "" && right_string != "") + if (left_deps.size () == 0 && right_deps.size () > 0) return ascending; - if (left_string != "" && right_string == "") + if (left_deps.size () > 0 && right_deps.size () == 0) return !ascending; // Sort on the first dependency. - left_number = Context::getContext ().tdb2.id (left_string.substr (0, 36)); - right_number = Context::getContext ().tdb2.id (right_string.substr (0, 36)); + left_number = Context::getContext ().tdb2.id (left_deps[0].substr (0, 36)); + right_number = Context::getContext ().tdb2.id (right_deps[0].substr (0, 36)); if (left_number == right_number) continue;