diff --git a/ChangeLog b/ChangeLog index 6f260db33..826455575 100644 --- a/ChangeLog +++ b/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, diff --git a/doc/man/taskrc.5.in b/doc/man/taskrc.5.in index f628f3b60..b8c14c70e 100644 --- a/doc/man/taskrc.5.in +++ b/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 diff --git a/src/Context.cpp b/src/Context.cpp index 41c1b656b..b0499bb9b 100644 --- a/src/Context.cpp +++ b/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" diff --git a/src/Task.cpp b/src/Task.cpp index 081f4b373..cc0316c9d 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -199,7 +199,7 @@ bool Task::has (const std::string& name) const } //////////////////////////////////////////////////////////////////////////////// -std::vector Task::all () +std::vector Task::all () const { std::vector 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":["",""] - // With json.depends.array=off "depends":"," - // - // 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"); - if (depends.find (uuid) != std::string::npos) + // the addDependency(&std::string) overload will check this, too, but here we + // 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. - std::string depends = get ("depends"); - if (depends != "") + if (hasDependency (uuid)) { - // 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); - if (i != deps.end ()) - { - deps.erase (i); - set ("depends", join (",", deps)); - recalc_urgency = true; - } - else - throw format ("Could not delete a dependency on task {1} - not found.", uuid); + // The removeDependency(std::string&) method will check this too, but here we + // can give a more natural error message containing the id provided by the user + if (uuid == "" || !has (dep2Attr (uuid))) + throw format ("Could not delete a dependency on task {1} - not found.", id); + removeDependency (uuid); } //////////////////////////////////////////////////////////////////////////////// -void Task::removeDependency (int id) +void Task::removeDependency (const std::string& uuid) { - std::string depends = get ("depends"); - std::string uuid = Context::getContext ().tdb2.pending.uuid (id); - if (uuid != "" && depends.find (uuid) != std::string::npos) - removeDependency (uuid); + auto depattr = dep2Attr (uuid); + if (has (depattr)) + remove (depattr); 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 i = std::find (deps.begin (), deps.end (), uuid); - return i != deps.end (); + auto depattr = dep2Attr (uuid); + return has (depattr); } //////////////////////////////////////////////////////////////////////////////// std::vector Task::getDependencyIDs () const { - std::vector all; - for (auto& dep : split (get ("depends"), ',')) - all.push_back (Context::getContext ().tdb2.pending.id (dep)); + std::vector ids; + for (auto& attr : all ()) { + if (!isDepAttr (attr)) + continue; + auto dep = attr2Dep (attr); + ids.push_back (Context::getContext ().tdb2.pending.id (dep)); + } - return all; + return ids; } //////////////////////////////////////////////////////////////////////////////// std::vector Task::getDependencyUUIDs () const { - return split (get ("depends"), ','); + std::vector uuids; + for (auto& attr : all ()) { + if (!isDepAttr (attr)) + continue; + auto dep = attr2Dep (attr); + uuids.push_back (dep); + } + + return uuids; } //////////////////////////////////////////////////////////////////////////////// std::vector 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 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::getBlockedTasks () const 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) + it.hasDependency (uuid)) 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. diff --git a/src/Task.h b/src/Task.h index 5dbc679d5..492f4c92a 100644 --- a/src/Task.h +++ b/src/Task.h @@ -88,7 +88,7 @@ public: void setAsNow (const std::string&); bool has (const std::string&) const; - std::vector all (); + std::vector 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: diff --git a/src/commands/CmdInfo.cpp b/src/commands/CmdInfo.cpp index 25115aaf4..b003e3db0 100644 --- a/src/commands/CmdInfo.cpp +++ b/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 (); diff --git a/src/commands/CmdShow.cpp b/src/commands/CmdShow.cpp index ae5b88816..38421ed2d 100644 --- a/src/commands/CmdShow.cpp +++ b/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" diff --git a/src/sort.cpp b/src/sort.cpp index 92056c366..7bac97047 100644 --- a/src/sort.cpp +++ b/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)); - right_number = Context::getContext ().tdb2.id (right_deps[0].substr (0, 36)); + left_number = Context::getContext ().tdb2.id (left_deps[0]); + right_number = Context::getContext ().tdb2.id (right_deps[0]); if (left_number == right_number) continue; diff --git a/test/export.t b/test/export.t index 90670f44a..6d970a8b6 100755 --- a/test/export.t +++ b/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') diff --git a/test/import.t b/test/import.t index 1a18415ad..eb5b48f42 100755 --- a/test/import.t +++ b/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)