Refactor to store dependencies as individual attributes

This also drops support for the transitional `json.depends.array`
configuration value, which has not been necessary since ~2016.

As with tags, dependencies are stored in both a "combined",
comma-separated format (for compatibility) and in an
attribute-per-dependency format (for the future).

ChangeLog

@@ -1,5 +1,14 @@
 2.6.0 () -
 
+- TW #2569 The `json.depends.array` configuration option is now ignored.
+           Dependencies are always represented as an array in JSON output.
+- TW #2554 Waiting is now an entirely "virtual" concept, based on a task's
+           'wait' property and the current time.  This is reflected in the +WAITING
+           tag, and in the now-deprecated `waiting` status.  Please upgrade filters
+           and other automation to use `+WAITING` or `wait.after:now` instead of
+           `status:waiting`, as support will be dropped in a future version.
+           TaskWarrior no longer explicitly "unwaits" a task, so the "unwait' verbosity
+           token is no longer available.
 - TW #1654 "Due" parsing behaviour seems inconsistent
            Thanks to Max Rossmannek.
 - TW #1788 When deleting recurring task all tasks, including completed tasks,

doc/man/taskrc.5.in

@@ -424,12 +424,6 @@ array.
 With json.array=0, export writes raw JSON objects to STDOUT, one per line.
 Defaults to "1".
 
-.TP
-.B json.depends.array=1
-Determines whether the export command encodes dependencies as an array of string
-UUIDs, or one comma-separated string.
-Defaults to "1".
-
 .TP
 .B _forcecolor=1
 Taskwarrior shuts off color automatically when the output is not sent directly

src/Context.cpp

@@ -109,7 +109,6 @@ std::string configurationDefaults =
   "xterm.title=0                                  # Sets xterm title for some commands\n"
   "expressions=infix                              # Prefer infix over postfix expressions\n"
   "json.array=1                                   # Enclose JSON output in [ ]\n"
-  "json.depends.array=0                           # Encode dependencies as a JSON array\n"
   "abbreviation.minimum=2                         # Shortest allowed abbreviation\n"
   "\n"
   "# Dates\n"

src/Task.cpp

@@ -199,7 +199,7 @@ bool Task::has (const std::string& name) const
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-std::vector <std::string> Task::all ()
+std::vector <std::string> Task::all () const
 {
   std::vector <std::string> all;
   for (const auto& i : data)
@@ -672,6 +672,14 @@ void Task::parse (const std::string& input)
   // ..and similarly, update `tags` to match the `tag_..` attributes
   fixTagsAttribute();
 
+  // same for `depends` / `dep_..`
+  if (data.find ("depends") != data.end ()) {
+    for (auto& dep : split(data["depends"], ',')) {
+      data[dep2Attr(dep)] = "x";
+    }
+  }
+  fixDependsAttribute();
+
   recalc_urgency = true;
 }
 
@@ -938,8 +946,10 @@ std::string Task::composeJSON (bool decorate /*= false*/) const
     if (! i.first.compare (0, 11, "annotation_", 11))
       continue;
 
+    // Tags and dependencies are handled below
     if (i.first == "tags" || isTagAttr (i.first))
-      // Tags are handled below
+      continue;
+    if (i.first == "depends" || isDepAttr (i.first))
       continue;
 
     // If value is an empty string, do not ever output it
@@ -983,45 +993,6 @@ std::string Task::composeJSON (bool decorate /*= false*/) const
       ++attributes_written;
     }
 
-    // Dependencies are an array by default.
-    else if (i.first == "depends"
-#ifdef PRODUCT_TASKWARRIOR
-    // 2016-02-20: Taskwarrior 2.5.0 introduced the 'json.depends.array' setting
-    //             which defaulted to 'on', and emitted this JSON for
-    //             dependencies:
-    //
-    //             With json.depends.array=on    "depends":["<uuid>","<uuid>"]
-    //             With json.depends.array=off   "depends":"<uuid>,<uuid>"
-    //
-    //             Taskwarrior 2.5.1 defaults this to 'off', because Taskserver
-    //             1.0.0 and 1.1.0 both expect that. Taskserver 1.2.0 will
-    //             accept both forms, but emit the 'off' variant.
-    //
-    //             When Taskwarrior 2.5.0 is no longer the dominant version,
-    //             and Taskserver 1.2.0 is released, the default for
-    //             'json.depends.array' can revert to 'on'.
-
-             && Context::getContext ().config.getBoolean ("json.depends.array")
-#endif
-            )
-    {
-      auto deps = split (i.second, ',');
-
-      out << "\"depends\":[";
-
-      int count = 0;
-      for (const auto& i : deps)
-      {
-        if (count++)
-          out << ',';
-
-        out << '"' << i << '"';
-      }
-
-      out << ']';
-      ++attributes_written;
-    }
-
     // Everything else is a quoted value.
     else
     {
@@ -1082,6 +1053,25 @@ std::string Task::composeJSON (bool decorate /*= false*/) const
     ++attributes_written;
   }
 
+  auto depends = getDependencyUUIDs ();
+  if (depends.size() > 0)
+  {
+    out << ','
+        << "\"depends\":[";
+
+    int count = 0;
+    for (const auto& dep : depends)
+    {
+      if (count++)
+        out << ',';
+
+      out << '"' << dep << '"';
+    }
+
+    out << ']';
+    ++attributes_written;
+  }
+
 #ifdef PRODUCT_TASKWARRIOR
   // Include urgency.
   if (decorate)
@@ -1186,8 +1176,10 @@ void Task::addDependency (int depid)
   if (uuid == "")
     throw format ("Could not create a dependency on task {1} - not found.", depid);
 
-  std::string depends = get ("depends");
+  // the addDependency(&std::string) overload will check this, too, but here we
-  if (depends.find (uuid) != std::string::npos)
+  // can give an more natural error message containing the id the user
+  // provided.
+  if (hasDependency (uuid))
   {
     Context::getContext ().footnote (format ("Task {1} already depends on task {2}.", id, depid));
     return;
@@ -1203,23 +1195,16 @@ void Task::addDependency (const std::string& uuid)
   if (uuid == get ("uuid"))
     throw std::string ("A task cannot be dependent on itself.");
 
-  // Store the dependency.
+  if (hasDependency (uuid))
-  std::string depends = get ("depends");
-  if (depends != "")
   {
-    // Check for extant dependency.
-    if (depends.find (uuid) == std::string::npos)
-      set ("depends", depends + ',' + uuid);
-    else
-    {
 #ifdef PRODUCT_TASKWARRIOR
-      Context::getContext ().footnote (format ("Task {1} already depends on task {2}.", get ("uuid"), uuid));
+    Context::getContext ().footnote (format ("Task {1} already depends on task {2}.", get ("uuid"), uuid));
 #endif
-      return;
+    return;
-    }
   }
-  else
+
-    set ("depends", uuid);
+  // Store the dependency.
+  set (dep2Attr (uuid), "x");
 
   // Prevent circular dependencies.
 #ifdef PRODUCT_TASKWARRIOR
@@ -1228,75 +1213,84 @@ void Task::addDependency (const std::string& uuid)
 #endif
 
   recalc_urgency = true;
+  fixDependsAttribute();
 }
 
 #ifdef PRODUCT_TASKWARRIOR
 ////////////////////////////////////////////////////////////////////////////////
-void Task::removeDependency (const std::string& uuid)
+void Task::removeDependency (int id)
 {
-  auto deps = split (get ("depends"), ',');
+  std::string uuid = Context::getContext ().tdb2.pending.uuid (id);
 
-  auto i = std::find (deps.begin (), deps.end (), uuid);
+  // The removeDependency(std::string&) method will check this too, but here we
-  if (i != deps.end ())
+  // can give a more natural error message containing the id provided by the user
-  {
+  if (uuid == "" || !has (dep2Attr (uuid)))
-    deps.erase (i);
+    throw format ("Could not delete a dependency on task {1} - not found.", id);
-    set ("depends", join (",", deps));
+  removeDependency (uuid);
-    recalc_urgency = true;
-  }
-  else
-    throw format ("Could not delete a dependency on task {1} - not found.", uuid);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-void Task::removeDependency (int id)
+void Task::removeDependency (const std::string& uuid)
 {
-  std::string depends = get ("depends");
+  auto depattr = dep2Attr (uuid);
-  std::string uuid = Context::getContext ().tdb2.pending.uuid (id);
+  if (has (depattr))
-  if (uuid != "" && depends.find (uuid) != std::string::npos)
+    remove (depattr);
-    removeDependency (uuid);
   else
-    throw format ("Could not delete a dependency on task {1} - not found.", id);
+    throw format ("Could not delete a dependency on task {1} - not found.", uuid);
+
+  recalc_urgency = true;
+  fixDependsAttribute();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 bool Task::hasDependency (const std::string& uuid) const
 {
-  auto deps = split (get ("depends"), ',');
+  auto depattr = dep2Attr (uuid);
-
+  return has (depattr);
-  auto i = std::find (deps.begin (), deps.end (), uuid);
-  return i != deps.end ();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 std::vector <int> Task::getDependencyIDs () const
 {
-  std::vector <int> all;
+  std::vector <int> ids;
-  for (auto& dep : split (get ("depends"), ','))
+  for (auto& attr : all ()) {
-    all.push_back (Context::getContext ().tdb2.pending.id (dep));
+    if (!isDepAttr (attr))
+      continue;
+    auto dep = attr2Dep (attr);
+    ids.push_back (Context::getContext ().tdb2.pending.id (dep));
+  }
 
-  return all;
+  return ids;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 std::vector <std::string> Task::getDependencyUUIDs () const
 {
-  return split (get ("depends"), ',');
+  std::vector <std::string> uuids;
+  for (auto& attr : all ()) {
+    if (!isDepAttr (attr))
+      continue;
+    auto dep = attr2Dep (attr);
+    uuids.push_back (dep);
+  }
+
+  return uuids;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 std::vector <Task> Task::getDependencyTasks () const
 {
-  auto depends = get ("depends");
+  auto uuids = getDependencyUUIDs ();
 
   // 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 != "")
+  if (uuids.size() > 0)
     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)
+          std::find (uuids.begin (), uuids.end (), it.get ("uuid")) != uuids.end ())
         blocking.push_back (it);
 
   return blocking;
@@ -1311,8 +1305,7 @@ std::vector <Task> Task::getBlockedTasks () const
   for (auto& it : Context::getContext ().tdb2.pending.get_tasks ())
     if (it.getStatus () != Task::completed &&
         it.getStatus () != Task::deleted   &&
-        it.has ("depends")                 &&
+        it.hasDependency (uuid))
-        it.get ("depends").find (uuid) != std::string::npos)
       blocked.push_back (it);
 
   return blocked;
@@ -1497,6 +1490,40 @@ const std::string Task::attr2Tag (const std::string& attr) const
   return attr.substr(5);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+void Task::fixDependsAttribute ()
+{
+  // Fix up the old `depends` attribute to match the `dep_..` attributes (or
+  // remove it if there are no deps)
+  auto deps = getDependencyUUIDs ();
+  if (deps.size () > 0) {
+    set ("depends", join (",", deps));
+  } else {
+    remove ("depends");
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+bool Task::isDepAttr(const std::string& attr) const
+{
+  return attr.compare(0, 4, "dep_") == 0;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+const std::string Task::dep2Attr (const std::string& tag) const
+{
+  std::stringstream tag_attr;
+  tag_attr << "dep_" << tag;
+  return tag_attr.str();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+const std::string Task::attr2Dep (const std::string& attr) const
+{
+  assert (isDepAttr (attr));
+  return attr.substr(4);
+}
+
 #ifdef PRODUCT_TASKWARRIOR
 ////////////////////////////////////////////////////////////////////////////////
 // A UDA Orphan is an attribute that is not represented in context.columns.

src/Task.h

@@ -88,7 +88,7 @@ public:
 
   void setAsNow (const std::string&);
   bool has (const std::string&) const;
-  std::vector <std::string> all ();
+  std::vector <std::string> all () const;
   const std::string identifier (bool shortened = false) const;
   const std::string get (const std::string&) const;
   const std::string& get_ref (const std::string&) const;
@@ -176,6 +176,10 @@ private:
   bool isTagAttr (const std::string&) const;
   const std::string tag2Attr (const std::string&) const;
   const std::string attr2Tag (const std::string&) const;
+  bool isDepAttr (const std::string&) const;
+  const std::string dep2Attr (const std::string&) const;
+  const std::string attr2Dep (const std::string&) const;
+  void fixDependsAttribute ();
   void fixTagsAttribute ();
 
 public:

src/commands/CmdInfo.cpp

@@ -414,6 +414,7 @@ int CmdInfo::execute (std::string& output)
     {
       if (att.substr (0, 11) != "annotation_" &&
           att.substr (0, 5) != "tags_" &&
+          att.substr (0, 4) != "dep_" &&
           Context::getContext ().columns.find (att) == Context::getContext ().columns.end ())
       {
          row = view.addRow ();

src/commands/CmdShow.cpp

@@ -172,7 +172,6 @@ int CmdShow::execute (std::string& output)
     " journal.time.start.annotation"
     " journal.time.stop.annotation"
     " json.array"
-    " json.depends.array"
     " list.all.projects"
     " list.all.tags"
     " locking"

src/sort.cpp

@@ -217,8 +217,8 @@ static bool sort_compare (int left, int right)
         return !ascending;
 
       // Sort on the first dependency.
-      left_number  = Context::getContext ().tdb2.id (left_deps[0].substr (0, 36));
+      left_number  = Context::getContext ().tdb2.id (left_deps[0]);
-      right_number = Context::getContext ().tdb2.id (right_deps[0].substr (0, 36));
+      right_number = Context::getContext ().tdb2.id (right_deps[0]);
 
       if (left_number == right_number)
         continue;

test/export.t

@@ -146,7 +146,6 @@ class TestExportCommand(TestCase):
         self.t(('add', 'everything depends on me task'))
         self.t(('add', 'wrong, everything depends on me task'))
         self.t('1 modify depends:2,3')
-        self.t.config('json.depends.array', 'on')
 
         deps = self.export(1)['depends']
         self.assertType(deps, list)
@@ -155,19 +154,6 @@ class TestExportCommand(TestCase):
         for uuid in deps:
             self.assertString(uuid, UUID_REGEXP, regexp=True)
 
-    def test_export_depends_oldformat(self):
-        self.t(('add', 'everything depends on me task'))
-        self.t(('add', 'wrong, everything depends on me task'))
-        self.t('1 modify depends:2,3')
-
-        code, out, err = self.t("rc.json.array=off rc.json.depends.array=off 1 export")
-        deps = json.loads(out)["depends"]
-        self.assertString(deps)
-        self.assertEqual(len(deps.split(",")), 2)
-
-        for uuid in deps.split(','):
-            self.assertString(uuid, UUID_REGEXP, regexp=True)
-
     def test_export_urgency(self):
         self.t('add urgent task +urgent')
 

test/import.t

@@ -187,15 +187,6 @@ class TestImport(TestCase):
         self.assertIn("Imported 3 tasks", err)
         self.assertData1()
 
-    def test_import_old_depend(self):
-        """One dependency used to be a plain string"""
-        _data = """{"uuid":"a0000000-a000-a000-a000-a00000000000","depends":"a1111111-a111-a111-a111-a11111111111","description":"zero","project":"A","status":"pending","entry":"1234567889"}"""
-        self.t("import", input=self.data1)
-        self.t("import", input=_data)
-        self.t.config("json.depends.array", "0")
-        _t = self.t.export("a0000000-a000-a000-a000-a00000000000")[0]
-        self.assertEqual(_t["depends"], "a1111111-a111-a111-a111-a11111111111")
-
     def test_import_old_depends(self):
         """Several dependencies used to be a comma seperated string"""
         _data = """{"uuid":"a0000000-a000-a000-a000-a00000000000","depends":"a1111111-a111-a111-a111-a11111111111,a2222222-a222-a222-a222-a22222222222","description":"zero","project":"A","status":"pending","entry":"1234567889"}"""
@@ -207,7 +198,6 @@ class TestImport(TestCase):
 
     def test_import_new_depend(self):
         """One dependency is a single array element"""
-        self.t.config('json.depends.array', 'on')
         _data = """{"uuid":"a0000000-a000-a000-a000-a00000000000","depends":["a1111111-a111-a111-a111-a11111111111"],"description":"zero","project":"A","status":"pending","entry":"1234567889"}"""
         self.t("import", input=self.data1)
         self.t("import", input=_data)
@@ -216,7 +206,6 @@ class TestImport(TestCase):
 
     def test_import_new_depends(self):
         """Several dependencies are an array"""
-        self.t.config('json.depends.array', 'on')
         _data = """{"uuid":"a0000000-a000-a000-a000-a00000000000","depends":["a1111111-a111-a111-a111-a11111111111","a2222222-a222-a222-a222-a22222222222"],"description":"zero","project":"A","status":"pending","entry":"1234567889"}"""
         self.t("import", input=self.data1)
         self.t("import", input=_data)