Remove references to the 'depends' property outside of Task.cpp

With the exception of `taskDifferences` and `taskInfoDifferences`,
deferred to #2572.

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;
         }
       }
     }

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 <int> Task::getDependencyIDs () const
 {
@@ -1277,15 +1286,36 @@ std::vector <std::string> Task::getDependencyUUIDs () const
 ////////////////////////////////////////////////////////////////////////////////
 std::vector <Task> Task::getDependencyTasks () const
 {
-  std::vector <Task> 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 <Task> 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> Task::getBlockedTasks () const
+{
+  auto uuid = get ("uuid");
+
+  std::vector <Task> 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 ());

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 <int>         getDependencyIDs () const;
   std::vector <std::string> getDependencyUUIDs () const;
+  std::vector <Task>        getBlockedTasks () const;
   std::vector <Task>        getDependencyTasks () const;
 
   std::vector <std::string> getUDAOrphanUUIDs () const;

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 <int> 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 <int>(dependencyGetBlocking (task).size ())) + ']');
+      renderStringRight (lines, width, color, '[' + format (static_cast <int>(deptasks.size ())) + ']');
     }
 
     else if (_style == "default" ||
              _style == "list")
     {
-      auto blocking = dependencyGetBlocking (task);
-
       std::vector <int> 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);

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)

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;

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<Task&>(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);

src/dependency.cpp

@@ -36,38 +36,6 @@
 
 #define STRING_DEPEND_BLOCKED        "Task {1} is blocked by:"
 
-////////////////////////////////////////////////////////////////////////////////
-std::vector <Task> dependencyGetBlocked (const Task& task)
-{
-  auto uuid = task.get ("uuid");
-
-  std::vector <Task> 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 <Task> dependencyGetBlocking (const Task& task)
-{
-  auto depends = task.get ("depends");
-
-  std::vector <Task> 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.

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)

src/main.h

@@ -59,8 +59,6 @@ std::string colorizeError (const std::string&);
 std::string colorizeDebug (const std::string&);
 
 // dependency.cpp
-std::vector <Task> dependencyGetBlocked (const Task&);
-std::vector <Task> dependencyGetBlocking (const Task&);
 bool dependencyIsCircular (const Task&);
 void dependencyChainOnComplete (Task&);
 void dependencyChainOnStart (Task&);

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;