From 901283c79f6e2ed08873835081ce7e265f7ecfdf Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 8 Aug 2021 02:08:43 +0000 Subject: [PATCH 01/44] [WIP] make 'waiting' status a 'virtual' status --- ChangeLog | 7 +++++++ doc/man/taskrc.5.in | 5 ++--- src/Context.cpp | 5 ++--- src/TDB2.cpp | 18 ------------------ src/Task.cpp | 40 ++++++++++++++++++++++++++++++++++++---- src/Task.h | 1 + test/wait.t | 6 ++---- 7 files changed, 50 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index 07ca4bb1a..94fe52489 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2.6.0 () - +- 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 3c22e9519..f628f3b60 100644 --- a/doc/man/taskrc.5.in +++ b/doc/man/taskrc.5.in @@ -299,11 +299,10 @@ control specific occasions when output is generated. This list may contain: sync Feedback about sync filter Shows the filter used in the command context Show the current context. Displayed in footnote. - unwait Notification when a task leaves the 'waiting' state override Notification when configuration options are overridden recur Notification when a new recurring task instance is created -"affected", "new-id", "new-uuid", "project", "unwait", "override" and "recur" +"affected", "new-id", "new-uuid", "project", "override" and "recur" imply "footnote". Note that the "1" setting is equivalent to all the tokens being specified, @@ -312,7 +311,7 @@ and the "nothing" setting is equivalent to none of the tokens being specified. Here are the shortcut equivalents: verbose=on - verbose=blank,header,footnote,label,new-id,affected,edit,special,project,sync,filter,unwait,override,recur + verbose=blank,header,footnote,label,new-id,affected,edit,special,project,sync,filter,override,recur verbose=0 verbose=blank,label,new-id,edit diff --git a/src/Context.cpp b/src/Context.cpp index fc7cfe130..085c2a87c 100644 --- a/src/Context.cpp +++ b/src/Context.cpp @@ -87,7 +87,7 @@ std::string configurationDefaults = "\n" "# Miscellaneous\n" "# # Comma-separated list. May contain any subset of:\n" - "verbose=blank,header,footnote,label,new-id,affected,edit,special,project,sync,filter,unwait,override,recur\n" + "verbose=blank,header,footnote,label,new-id,affected,edit,special,project,sync,filter,override,recur\n" "confirmation=1 # Confirmation on delete, big changes\n" "recurrence=1 # Enable recurrence\n" "recurrence.confirmation=prompt # Confirmation for propagating changes among recurring tasks (yes/no/prompt)\n" @@ -1029,7 +1029,6 @@ bool Context::verbose (const std::string& token) v != "project" && // v != "sync" && // v != "filter" && // - v != "unwait" && // v != "override" && // v != "context" && // v != "recur") // @@ -1043,7 +1042,7 @@ bool Context::verbose (const std::string& token) if (! verbosity.count ("footnote")) { // TODO: Some of these may not use footnotes yet. They should. - for (auto flag : {"affected", "new-id", "new-uuid", "project", "unwait", "override", "recur"}) + for (auto flag : {"affected", "new-id", "new-uuid", "project", "override", "recur"}) { if (verbosity.count (flag)) { diff --git a/src/TDB2.cpp b/src/TDB2.cpp index 21234ebc1..9b1cf7443 100644 --- a/src/TDB2.cpp +++ b/src/TDB2.cpp @@ -355,23 +355,6 @@ void TF2::load_gc (Task& task) { Context::getContext ().tdb2.pending._tasks.push_back (task); } - else if (status == "waiting") - { - Datetime wait (task.get_date ("wait")); - if (wait < now) - { - task.set ("status", "pending"); - task.remove ("wait"); - // Unwaiting pending tasks is the only case not caught by the size() - // checks in TDB2::gc(), so we need to signal it here. - Context::getContext ().tdb2.pending._dirty = true; - - if (Context::getContext ().verbose ("unwait")) - Context::getContext ().footnote (format ("Un-waiting task {1} '{2}'", task.id, task.get ("description"))); - } - - Context::getContext ().tdb2.pending._tasks.push_back (task); - } else { Context::getContext ().tdb2.completed._tasks.push_back (task); @@ -1249,7 +1232,6 @@ void TDB2::show_diff ( // Possible scenarios: // - task in pending that needs to be in completed // - task in completed that needs to be in pending -// - waiting task in pending that needs to be un-waited void TDB2::gc () { Timer timer; diff --git a/src/Task.cpp b/src/Task.cpp index e7c6c377a..83e60def3 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -147,7 +147,9 @@ Task::status Task::textToStatus (const std::string& input) else if (input[0] == 'c') return Task::completed; else if (input[0] == 'd') return Task::deleted; else if (input[0] == 'r') return Task::recurring; - else if (input[0] == 'w') return Task::waiting; + // for compatibility, parse `w` as pending; Task::getStatus will + // apply the virtual waiting status if appropriate + else if (input[0] == 'w') return Task::pending; throw format ("The status '{1}' is not valid.", input); } @@ -301,12 +303,25 @@ Task::status Task::getStatus () const if (! has ("status")) return Task::pending; - return textToStatus (get ("status")); + auto status = textToStatus (get ("status")); + + // implement the "virtual" Task::waiting status, which is not stored on-disk + // but is defined as a pending task with a `wait` attribute in the future. + if (status == Task::pending && is_waiting ()) { + return Task::waiting; + } + + return status; } //////////////////////////////////////////////////////////////////////////////// void Task::setStatus (Task::status status) { + // the 'waiting' status is a virtual version of 'pending', so translate + // that back to 'pending' here + if (status == Task::waiting) + status = Task::pending; + set ("status", statusToText (status)); recalc_urgency = true; @@ -559,6 +574,23 @@ bool Task::is_overdue () const } #endif +//////////////////////////////////////////////////////////////////////////////// +bool Task::is_waiting () const +{ + // note that is_waiting can return true for tasks in an actual status other + // than pending; in this case +WAITING will be set but the status will not be + // "waiting" + if (has ("wait")) + { + Datetime now; + Datetime wait (get_date ("wait")); + if (wait > now) + return true; + } + + return false; +} + //////////////////////////////////////////////////////////////////////////////// // Attempt an FF4 parse first, using Task::parse, and in the event of an error // try a JSON parse, otherwise a legacy parse (currently no legacy formats are @@ -1311,7 +1343,7 @@ bool Task::hasTag (const std::string& tag) const if (tag == "TAGGED") return getTagCount() > 0; if (tag == "PARENT") return has ("mask") || has ("last"); // 2017-01-07: Deprecated in 2.6.0 if (tag == "TEMPLATE") return has ("last") || has ("mask"); - if (tag == "WAITING") return get ("status") == "waiting"; + if (tag == "WAITING") return is_waiting (); if (tag == "PENDING") return get ("status") == "pending"; if (tag == "COMPLETED") return get ("status") == "completed"; if (tag == "DELETED") return get ("status") == "deleted"; @@ -2048,7 +2080,7 @@ float Task::urgency_scheduled () const //////////////////////////////////////////////////////////////////////////////// float Task::urgency_waiting () const { - if (get_ref ("status") == "waiting") + if (is_waiting ()) return 1.0; return 0.0; diff --git a/src/Task.h b/src/Task.h index 5112dc3f1..1936e2a50 100644 --- a/src/Task.h +++ b/src/Task.h @@ -114,6 +114,7 @@ public: bool is_udaPresent () const; bool is_orphanPresent () const; #endif + bool is_waiting () const; status getStatus () const; void setStatus (status); diff --git a/test/wait.t b/test/wait.t index 489a8318f..748dac78c 100755 --- a/test/wait.t +++ b/test/wait.t @@ -62,8 +62,6 @@ class TestWait(TestCase): self.assertIn("visible", out) self.assertIn("hidden", out) - self.assertIn("Un-waiting task 2 'hidden'", err) - class TestBug434(TestCase): # Bug #434: Task should not prevent users from marking as done tasks with @@ -106,7 +104,7 @@ class TestFeature2322(TestCase): self.t = Task() def test_done_unwait(self): - """2322: Done should un-wait a waiting task""" + """2322: Done should remove the wait attribute""" self.t("add foo wait:tomorrow") code, out, err = self.t("export") self.assertIn('"wait":', out) @@ -116,7 +114,7 @@ class TestFeature2322(TestCase): self.assertNotIn('"wait":', out) def test_delete_unwait(self): - """2322: Deleteion should un-wait a waiting task""" + """2322: Delete should remove the wait attribute""" self.t("add bar wait:tomorrow") code, out, err = self.t("export") self.assertIn('"wait":', out) From db324c41e3b5fa04862f5134070feccdc524c6b3 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 7 Aug 2021 23:36:35 -0400 Subject: [PATCH 02/44] TF2: Upgrade waiting tasks to pending, if applicable This can be safely removed in one of the later releases (likely 3.1 or later). --- src/TDB2.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/TDB2.cpp b/src/TDB2.cpp index 9b1cf7443..a5f8646ba 100644 --- a/src/TDB2.cpp +++ b/src/TDB2.cpp @@ -355,6 +355,14 @@ void TF2::load_gc (Task& task) { Context::getContext ().tdb2.pending._tasks.push_back (task); } + // 2.6.0: Waiting status is deprecated. Convert to pending to upgrade status + // field value in the data files. + else if (status == "waiting") + { + task.set ("status", "pending"); + Context::getContext ().tdb2.pending._tasks.push_back (task); + Context::getContext ().tdb2.pending._dirty = true; + } else { Context::getContext ().tdb2.completed._tasks.push_back (task); From 7d81eadd5a8fce33461d53d65766c8ed03114355 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 08:42:05 -0400 Subject: [PATCH 03/44] tests: Add test for the migration path from 2.5.3 --- test/tw-2563.t | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100755 test/tw-2563.t diff --git a/test/tw-2563.t b/test/tw-2563.t new file mode 100755 index 000000000..1f8d0bb9a --- /dev/null +++ b/test/tw-2563.t @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# This tests the migration path from 2.5.3 or earlier to 2.6.0 with respect to +# the upgrade of the status field from waiting to pending +. bash_tap_tw.sh + +# Setup +task add Actionable task wait:yesterday +task add Non-actionable task wait:tomorrow+1h + +# Simulate this was created in 2.5.3 or earlier (status is equal to waiting, +# not pending) +sed -i 's/pending/waiting/' $TASKDATA/pending.data + +# Trigger upgrade +task all + +# Assertion: Exactly one task is considered waiting +[[ `task +WAITING count` == "1" ]] + +# Report file content +echo pending.data +cat $TASKDATA/pending.data +echo completed.data +cat $TASKDATA/completed.data + +# Assertion: No lines in data files with "waiting" status +[[ -z `cat $TASKDATA/pending.data | grep waiting` ]] +[[ -z `cat $TASKDATA/completed.data | grep waiting` ]] + +# Assertion: No tasks were moved into completed.data +[[ `cat $TASKDATA/pending.data | wc -l` == "2" ]] +[[ `cat $TASKDATA/completed.data | wc -l` == "0" ]] From 582bee66e903f0e19550296d3db7c64563a683a0 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 09:24:54 -0400 Subject: [PATCH 04/44] DOM: Implement special-cased status handling This is required for status:pending filters not matching the tasks with the virutal waiting tag. --- src/DOM.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/DOM.cpp b/src/DOM.cpp index 17c693c43..4cfa01a9a 100644 --- a/src/DOM.cpp +++ b/src/DOM.cpp @@ -330,6 +330,14 @@ bool getDOM (const std::string& name, const Task& task, Variant& value) return true; } + // Special handling of status required for virtual waiting status + // implementation + if (ref.data.size () && size == 1 && canonical == "status") + { + value = Variant (ref.statusToText (ref.getStatus ())); + return true; + } + Column* column = Context::getContext ().columns[canonical]; if (ref.data.size () && size == 1 && column) From 0c22823771f7ffd01e3ee780ddde75e9ad431d72 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 09:35:17 -0400 Subject: [PATCH 05/44] tests: Ensure virtual tags +PENDING and +WAITING are working as before TW-2563 --- test/tw-2563.t | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/tw-2563.t b/test/tw-2563.t index 1f8d0bb9a..2636fc2b8 100755 --- a/test/tw-2563.t +++ b/test/tw-2563.t @@ -14,15 +14,26 @@ sed -i 's/pending/waiting/' $TASKDATA/pending.data # Trigger upgrade task all -# Assertion: Exactly one task is considered waiting -[[ `task +WAITING count` == "1" ]] - # Report file content echo pending.data cat $TASKDATA/pending.data echo completed.data cat $TASKDATA/completed.data +# Assertion: Exactly one task is considered waiting +[[ `task +WAITING count` == "1" ]] +[[ `task status:waiting count` == "1" ]] + +# Assertion: Exactly one task is considered pending +[[ `task +PENDING count` == "1" ]] +[[ `task status:pending count` == "1" ]] + +# Assertion: Task 1 is pending +[[ `task _get 1.status` == "pending" ]] + +# Assertion: Task 2 is waiting +[[ `task _get 2.status` == "waiting" ]] + # Assertion: No lines in data files with "waiting" status [[ -z `cat $TASKDATA/pending.data | grep waiting` ]] [[ -z `cat $TASKDATA/completed.data | grep waiting` ]] From ea008380dbeebae659fa922641057104baad6731 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 09:35:49 -0400 Subject: [PATCH 06/44] Task: Use getStatus () call to determine if task is pending --- src/Task.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Task.cpp b/src/Task.cpp index 83e60def3..60d898920 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -1344,7 +1344,7 @@ bool Task::hasTag (const std::string& tag) const if (tag == "PARENT") return has ("mask") || has ("last"); // 2017-01-07: Deprecated in 2.6.0 if (tag == "TEMPLATE") return has ("last") || has ("mask"); if (tag == "WAITING") return is_waiting (); - if (tag == "PENDING") return get ("status") == "pending"; + if (tag == "PENDING") return getStatus () == Task::pending; if (tag == "COMPLETED") return get ("status") == "completed"; if (tag == "DELETED") return get ("status") == "deleted"; #ifdef PRODUCT_TASKWARRIOR From 861e8a6414dbc0a23f8ff479f5184068415419d9 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 09:48:09 -0400 Subject: [PATCH 07/44] commands: Do not remove wait attribute upon task completion/deletion --- src/commands/CmdDelete.cpp | 4 ---- src/commands/CmdDone.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/commands/CmdDelete.cpp b/src/commands/CmdDelete.cpp index b6d88effd..894762154 100644 --- a/src/commands/CmdDelete.cpp +++ b/src/commands/CmdDelete.cpp @@ -92,10 +92,6 @@ int CmdDelete::execute (std::string&) if (! task.has ("end")) task.setAsNow ("end"); - // Un-wait the task, if waiting. - if (task.has ("wait")) - task.remove ("wait"); - if (permission (question, filtered.size ())) { updateRecurrenceMask (task); diff --git a/src/commands/CmdDone.cpp b/src/commands/CmdDone.cpp index 4f9b9f5ef..d9be84ccd 100644 --- a/src/commands/CmdDone.cpp +++ b/src/commands/CmdDone.cpp @@ -98,10 +98,6 @@ int CmdDone::execute (std::string&) task.addAnnotation (Context::getContext ().config.get ("journal.time.stop.annotation")); } - // Un-wait the task, if waiting. - if (task.has ("wait")) - task.remove ("wait"); - if (permission (taskDifferences (before, task) + question, filtered.size ())) { updateRecurrenceMask (task); From b6ce51e83df30159a28572c610ac3e0211556420 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 09:50:47 -0400 Subject: [PATCH 08/44] tests: Reverse expectations for wait attribute removal uponn done/delete The wait attribute is no longer expected to be removed by the done/delete commands. --- test/wait.t | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/wait.t b/test/wait.t index 748dac78c..484dc67dd 100755 --- a/test/wait.t +++ b/test/wait.t @@ -98,30 +98,30 @@ class Test1486(TestCase): self.assertNotIn('regular', out) -class TestFeature2322(TestCase): +class TestFeature2563(TestCase): def setUp(self): """Executed before each test in the class""" self.t = Task() def test_done_unwait(self): - """2322: Done should remove the wait attribute""" + """2563: Done should NOT remove the wait attribute""" self.t("add foo wait:tomorrow") code, out, err = self.t("export") self.assertIn('"wait":', out) self.t("1 done") code, out, err = self.t("export") - self.assertNotIn('"wait":', out) + self.assertIn('"wait":', out) def test_delete_unwait(self): - """2322: Delete should remove the wait attribute""" + """2563: Delete should NOT remove the wait attribute""" self.t("add bar wait:tomorrow") code, out, err = self.t("export") self.assertIn('"wait":', out) self.t("1 delete", input="y\n") code, out, err = self.t("export") - self.assertNotIn('"wait":', out) + self.assertIn('"wait":', out) if __name__ == "__main__": From 2c44f79277c8becabf33799ca962d7f38ad7caf1 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 10:00:31 -0400 Subject: [PATCH 09/44] tests: Make sed/wc calls in tw-2563 compatible with OS-X' The sed does not support -i flag in the same way. The wc -l prints whitespace in front, which needs to be stripped. --- test/tw-2563.t | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/tw-2563.t b/test/tw-2563.t index 2636fc2b8..e60c9dbb7 100755 --- a/test/tw-2563.t +++ b/test/tw-2563.t @@ -8,8 +8,9 @@ task add Actionable task wait:yesterday task add Non-actionable task wait:tomorrow+1h # Simulate this was created in 2.5.3 or earlier (status is equal to waiting, -# not pending) -sed -i 's/pending/waiting/' $TASKDATA/pending.data +# not pending). Using more cumbersome sed syntax for Mac OS-X compatibility. +sed -i".bak" 's/pending/waiting/g' $TASKDATA/pending.data +rm -f $TASKDATA/pending.data.bak # Trigger upgrade task all @@ -39,5 +40,6 @@ cat $TASKDATA/completed.data [[ -z `cat $TASKDATA/completed.data | grep waiting` ]] # Assertion: No tasks were moved into completed.data -[[ `cat $TASKDATA/pending.data | wc -l` == "2" ]] -[[ `cat $TASKDATA/completed.data | wc -l` == "0" ]] +cat $TASKDATA/pending.data | wc -l | tr -d ' ' +[[ `cat $TASKDATA/pending.data | wc -l | tr -d ' '` == "2" ]] +[[ `cat $TASKDATA/completed.data | wc -l | tr -d ' '` == "0" ]] From 91517151ad2708f68e0653c95e41f3c5e08fed60 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 10:10:06 -0400 Subject: [PATCH 10/44] docs: Add removal comments --- src/DOM.cpp | 2 +- src/Task.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/DOM.cpp b/src/DOM.cpp index 4cfa01a9a..b88c04894 100644 --- a/src/DOM.cpp +++ b/src/DOM.cpp @@ -331,7 +331,7 @@ bool getDOM (const std::string& name, const Task& task, Variant& value) } // Special handling of status required for virtual waiting status - // implementation + // implementation. Remove in 3.0.0. if (ref.data.size () && size == 1 && canonical == "status") { value = Variant (ref.statusToText (ref.getStatus ())); diff --git a/src/Task.cpp b/src/Task.cpp index 60d898920..4a7a63741 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -305,8 +305,9 @@ Task::status Task::getStatus () const auto status = textToStatus (get ("status")); - // implement the "virtual" Task::waiting status, which is not stored on-disk + // Implement the "virtual" Task::waiting status, which is not stored on-disk // but is defined as a pending task with a `wait` attribute in the future. + // This is workaround for 2.6.0, remove in 3.0.0. if (status == Task::pending && is_waiting ()) { return Task::waiting; } From f8ca8cff814667873a68204d23d4641ac1db9751 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 8 Aug 2021 12:00:21 -0400 Subject: [PATCH 11/44] Task: Make COMPLETED and DELETED virtual tags use getStatus() This is now consistent with how PENDING is defined since recent commit 520d7e979b17c90e91d29325be8294905b338ee6. --- src/Task.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Task.cpp b/src/Task.cpp index 4a7a63741..50b8e8bcd 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -1346,8 +1346,8 @@ bool Task::hasTag (const std::string& tag) const if (tag == "TEMPLATE") return has ("last") || has ("mask"); if (tag == "WAITING") return is_waiting (); if (tag == "PENDING") return getStatus () == Task::pending; - if (tag == "COMPLETED") return get ("status") == "completed"; - if (tag == "DELETED") return get ("status") == "deleted"; + if (tag == "COMPLETED") return getStatus () == Task::completed; + if (tag == "DELETED") return getStatus () == Task::deleted; #ifdef PRODUCT_TASKWARRIOR if (tag == "UDA") return is_udaPresent (); if (tag == "ORPHAN") return is_orphanPresent (); From 9a9ede7878e268074ecf1e3ec40ba5ac47b62e95 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 8 Aug 2021 17:50:52 +0000 Subject: [PATCH 12/44] update built-in reports to use +WAITING, and doc --- doc/man/task.1.in | 9 +++++---- src/Context.cpp | 24 ++++++++++++------------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/doc/man/task.1.in b/doc/man/task.1.in index 90e92cfce..95d952dd8 100644 --- a/doc/man/task.1.in +++ b/doc/man/task.1.in @@ -304,7 +304,7 @@ value. .B task ready Shows a page of the most urgent ready tasks, sorted by urgency with started tasks first. A ready task is one that is either unscheduled, or has a scheduled -date that is past and has no wait date. +date that is past and is not waiting. .TP .B task oldest @@ -796,9 +796,10 @@ to 25 lines. .TP .B wait: -When a task is given a wait date, it is hidden from most reports by changing -its status to 'waiting'. When that date is passed, the status is changed back -to 'pending', and the task becomes visible. +When a task is given a wait date, it is hidden from most built-in reports, which +exclude +WAITING. When the date is in the past, the task is not considered +WAITING, +and again becomes visible. Note that, for compatibilty, such tasks are shown as +having status "waiting", but this will change in a future release. .TP .B depends: diff --git a/src/Context.cpp b/src/Context.cpp index 085c2a87c..41c1b656b 100644 --- a/src/Context.cpp +++ b/src/Context.cpp @@ -292,43 +292,43 @@ std::string configurationDefaults = "report.long.description=All details of tasks\n" "report.long.labels=ID,A,Created,Mod,Deps,P,Project,Tags,Recur,Wait,Sched,Due,Until,Description\n" "report.long.columns=id,start.active,entry,modified.age,depends,priority,project,tags,recur,wait.remaining,scheduled,due,until,description\n" - "report.long.filter=status:pending\n" + "report.long.filter=status:pending -WAITING\n" "report.long.sort=modified-\n" "\n" "report.list.description=Most details of tasks\n" "report.list.labels=ID,Active,Age,D,P,Project,Tags,R,Sch,Due,Until,Description,Urg\n" "report.list.columns=id,start.age,entry.age,depends.indicator,priority,project,tags,recur.indicator,scheduled.countdown,due,until.remaining,description.count,urgency\n" - "report.list.filter=status:pending\n" + "report.list.filter=status:pending -WAITING\n" "report.list.sort=start-,due+,project+,urgency-\n" "\n" "report.ls.description=Few details of tasks\n" "report.ls.labels=ID,A,D,Project,Tags,R,Wait,S,Due,Until,Description\n" "report.ls.columns=id,start.active,depends.indicator,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due.countdown,until.countdown,description.count\n" - "report.ls.filter=status:pending\n" + "report.ls.filter=status:pending -WAITING\n" "report.ls.sort=start-,description+\n" "\n" "report.minimal.description=Minimal details of tasks\n" "report.minimal.labels=ID,Project,Tags,Description\n" "report.minimal.columns=id,project,tags.count,description.count\n" - "report.minimal.filter=status:pending or status:waiting\n" + "report.minimal.filter=status:pending\n" "report.minimal.sort=project+/,description+\n" "\n" "report.newest.description=Newest tasks\n" "report.newest.labels=ID,Active,Created,Age,Mod,D,P,Project,Tags,R,Wait,Sch,Due,Until,Description\n" "report.newest.columns=id,start.age,entry,entry.age,modified.age,depends.indicator,priority,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due,until.age,description\n" - "report.newest.filter=status:pending or status:waiting\n" + "report.newest.filter=status:pending\n" "report.newest.sort=entry-\n" "\n" "report.oldest.description=Oldest tasks\n" "report.oldest.labels=ID,Active,Created,Age,Mod,D,P,Project,Tags,R,Wait,Sch,Due,Until,Description\n" "report.oldest.columns=id,start.age,entry,entry.age,modified.age,depends.indicator,priority,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due,until.age,description\n" - "report.oldest.filter=status:pending or status:waiting\n" + "report.oldest.filter=status:pending\n" "report.oldest.sort=entry+\n" "\n" "report.overdue.description=Overdue tasks\n" "report.overdue.labels=ID,Active,Age,Deps,P,Project,Tag,R,S,Due,Until,Description,Urg\n" "report.overdue.columns=id,start.age,entry.age,depends,priority,project,tags,recur.indicator,scheduled.countdown,due,until,description,urgency\n" - "report.overdue.filter=(status:pending or status:waiting) and +OVERDUE\n" + "report.overdue.filter=status:pending and +OVERDUE\n" "report.overdue.sort=urgency-,due+\n" "\n" "report.active.description=Active tasks\n" @@ -346,7 +346,7 @@ std::string configurationDefaults = "report.recurring.description=Recurring Tasks\n" "report.recurring.labels=ID,Active,Age,D,P,Project,Tags,Recur,Sch,Due,Until,Description,Urg\n" "report.recurring.columns=id,start.age,entry.age,depends.indicator,priority,project,tags,recur,scheduled.countdown,due,until.remaining,description,urgency\n" - "report.recurring.filter=(status:pending or status:waiting) and (+PARENT or +CHILD)\n" + "report.recurring.filter=status:pending and (+PARENT or +CHILD)\n" "report.recurring.sort=due+,urgency-,entry+\n" "\n" "report.waiting.description=Waiting (hidden) tasks\n" @@ -363,7 +363,7 @@ std::string configurationDefaults = "report.next.description=Most urgent tasks\n" "report.next.labels=ID,Active,Age,Deps,P,Project,Tag,Recur,S,Due,Until,Description,Urg\n" "report.next.columns=id,start.age,entry.age,depends,priority,project,tags,recur,scheduled.countdown,due.relative,until.remaining,description,urgency\n" - "report.next.filter=status:pending limit:page\n" + "report.next.filter=status:pending -WAITING limit:page\n" "report.next.sort=urgency-\n" "\n" "report.ready.description=Most urgent actionable tasks\n" @@ -376,19 +376,19 @@ std::string configurationDefaults = "report.blocked.columns=id,depends,project,priority,due,start.active,entry.age,description\n" "report.blocked.labels=ID,Deps,Proj,Pri,Due,Active,Age,Description\n" "report.blocked.sort=due+,priority-,start-,project+\n" - "report.blocked.filter=status:pending +BLOCKED\n" + "report.blocked.filter=status:pending -WAITING +BLOCKED\n" "\n" "report.unblocked.description=Unblocked tasks\n" "report.unblocked.columns=id,depends,project,priority,due,start.active,entry.age,description\n" "report.unblocked.labels=ID,Deps,Proj,Pri,Due,Active,Age,Description\n" "report.unblocked.sort=due+,priority-,start-,project+\n" - "report.unblocked.filter=status:pending -BLOCKED\n" + "report.unblocked.filter=status:pending -WAITING -BLOCKED\n" "\n" "report.blocking.description=Blocking tasks\n" "report.blocking.labels=ID,UUID,A,Deps,Project,Tags,R,W,Sch,Due,Until,Description,Urg\n" "report.blocking.columns=id,uuid.short,start.active,depends,project,tags,recur,wait,scheduled.remaining,due.relative,until.remaining,description.count,urgency\n" "report.blocking.sort=urgency-,due+,entry+\n" - "report.blocking.filter=status:pending +BLOCKING\n" + "report.blocking.filter=status:pending -WAITING +BLOCKING\n" "\n" "report.timesheet.filter=(+PENDING and start.after:now-4wks) or (+COMPLETED and end.after:now-4wks)\n" "\n"; From b467049720e0ca4c79311a55fb0c572e81d4b9e7 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 14 Aug 2021 17:34:43 -0400 Subject: [PATCH 13/44] tests: Add tests for TW #1913 --- test/project.t | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/project.t b/test/project.t index d0dd84676..551bc2040 100755 --- a/test/project.t +++ b/test/project.t @@ -480,6 +480,26 @@ class TestBug1627(TestCase): self.assertEqual("mon\n", out) +class TestBug1900(TestCase): + def setUp(self): + """Executed before each test in the class""" + self.t = Task() + + def test_project_eval(self): + """1900: Project name can contain dashes""" + self.t("add foo project:due-b") + code, out, err = self.t("_get 1.project") + self.assertEqual("due-b\n", out) + + self.t("add foo project:scheduled-home") + code, out, err = self.t("_get 2.project") + self.assertEqual("scheduled-home\n", out) + + self.t("add foo project:entry-work") + code, out, err = self.t("_get 3.project") + self.assertEqual("entry-work\n", out) + + class TestBug1904(TestCase): def setUp(self): """Executed before each test in the class""" From b4fe3178966592a00e493500cc5d5f5fed123f99 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 14 Aug 2021 17:36:12 -0400 Subject: [PATCH 14/44] docs: Document TW #1913 --- ChangeLog | 2 ++ NEWS | 3 +++ 2 files changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 94fe52489..96d191a37 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,8 @@ Thanks to Matt Chun-Lum. - TW #1911 Support holidays longer then 1 day Thanks to Daniel Mowitz. +- TW #1913 Project names with dashes and attribute names fail to parse + Thanks to Yanick Champoux. - TW #1938 Missing annotation on import if entry is duplicated Thanks to Florian. - TW #1955 Adding tasks in context. diff --git a/NEWS b/NEWS index 4a150939b..ab900e217 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,9 @@ Fixed regressions in 2.6.0 the first configuration override. Otherwise task would by default inform about the other overrides (see #2247). This was a regression introduced in 2.5.2. + - The attribute values of the form "-", for + example "due-nextweek" or "scheduled-work" would fail to parse (see + #1913). This was a regression introduced in 2.5.1. Removed Features in 2.6.0 From 5a86a402202ee5498a8ba8b67689b446b228fc69 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 14 Aug 2021 17:48:17 -0400 Subject: [PATCH 15/44] tests: Temporarily disable testing on Debian stable --- .github/workflows/tests.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index cfbc6bae2..a7d2ced00 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -24,9 +24,6 @@ jobs: - name: "Fedora 34" runner: ubuntu-latest dockerfile: fedora34 - - name: "Debian Stable" - runner: ubuntu-latest - dockerfile: debianstable - name: "Debian Testing" runner: ubuntu-latest dockerfile: debiantesting From 89a6f2b62932f1125ba0eb8d67005eb2028999de Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 15 Aug 2021 16:32:27 -0400 Subject: [PATCH 16/44] CLI2: Apply UUID/ID context break only for readable context The purpose of this break is to not apply the context on commands like task 4 info so that we can still refer to tasks directly (using their ID/UUID references) even if they fall outside of the currectly active context. However, this break should not be applied for writeable context. This is because the lexer can (a bit misleadingly) label parts of the desription of the new task as number/identifier tokens task add Replace 3 parts of the puzzle abc123 ^ ^ type::number type:uuid which would trigger the break unnecessarily. Closes #2550. --- src/CLI2.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/CLI2.cpp b/src/CLI2.cpp index bcfe1feb3..5d9ce3666 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -607,17 +607,18 @@ void CLI2::addContext (bool readable, bool writeable) if (contextString.empty ()) return; - // Detect if UUID or ID is set, and bail out - for (auto& a : _args) - { - if (a._lextype == Lexer::Type::uuid || - a._lextype == Lexer::Type::number || - a._lextype == Lexer::Type::set) + // For readable contexts: Detect if UUID or ID is set, and bail out + if (readable) + for (auto& a : _args) { - Context::getContext ().debug (format ("UUID/ID argument found '{1}', not applying context.", a.attribute ("raw"))); - return; + if (a._lextype == Lexer::Type::uuid || + a._lextype == Lexer::Type::number || + a._lextype == Lexer::Type::set) + { + Context::getContext ().debug (format ("UUID/ID argument found '{1}', not applying context.", a.attribute ("raw"))); + return; + } } - } // Apply the context. Readable (filtering) takes precedence. Also set the // block now, since addFilter calls analyze(), which calls addContext(). From 7fb457d89252502b08e763fb295863a2eb02e4e5 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 15 Aug 2021 16:36:26 -0400 Subject: [PATCH 17/44] test: Add tests for TW #2550 --- test/tw-2550.t | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100755 test/tw-2550.t diff --git a/test/tw-2550.t b/test/tw-2550.t new file mode 100755 index 000000000..3a1e044ff --- /dev/null +++ b/test/tw-2550.t @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +. bash_tap_tw.sh + +# Setup the context +task config context.work.read '+work' +task config context.work.write '+work' + +# Create a task outside of the context +task add outside + +# Activate the context +task context work + +# Add multiple tasks within the context, some of which contain numbers or uuids +task add inside +task add inside 2 +task add inside 3 +task add inside aabbccdd +task add inside 4-5 + +# Assertion: Task defined outside of the context should not show up +[[ -z `task all | grep outside` ]] + +# Five tasks were defined within the context +task count +[[ `task count` == "5" ]] + +# Unset the context +task context none + +# Exactly five tasks have the tag work +task +work count +[[ `task +work count` == "5" ]] From d3fdb2baf97912659c87f7bab0f8f6922924ff42 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 15 Aug 2021 16:39:02 -0400 Subject: [PATCH 18/44] changelog: Adjust formatting --- ChangeLog | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 96d191a37..cde8d7808 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,8 @@ - TW #1804 Importing malformed annotation (without entry timestamp) causes segmentation fault. Thanks to David Badura. +- TW #1824 Fixed countdown formatting + Thanks to Sebastian Uharek - TW #1896 Parser cannot handle empty parentheses Thanks to Tomas Babej. - TW #1908 Cannot create task with explicit description 'start ....' @@ -76,6 +78,8 @@ Thanks to Scott Kostyshak. - TW #2503 Warn against executing an empty execute command. Thanks to heinrichat. +- TW #2208 Feature: added coloring of dates with scheduled tasks to calendar + Thanks to Sebastian Uharek - TW #2514 Duration values can be mis-reported in the task info output Thanks to reportaman. - TW #2519 Named date eod should be last minute of today and not first of @@ -86,10 +90,6 @@ - TW #2536 Feature: inclusive range-end attribute modifier 'by' so 'end of' named dates can be filtered inclusively Thanks to Scott Mcdermott -- TW #1824 Fixed countdown formatting - Thanks to Sebastian Uharek -- #2208 Feature: added coloring of dates with scheduled tasks to calendar - Thanks to Sebastian Uharek ------ current release --------------------------- From 54aef35b5785226138815e77fec3b114174b9495 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 15 Aug 2021 16:39:47 -0400 Subject: [PATCH 19/44] docs: Document #2550 --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index cde8d7808..4c94acde6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -90,6 +90,8 @@ - TW #2536 Feature: inclusive range-end attribute modifier 'by' so 'end of' named dates can be filtered inclusively Thanks to Scott Mcdermott +- TW #2550 Write context skipped if description contains an identifier + Thanks to Sebastian Fricke. ------ current release --------------------------- From 9e67f4f946d53bfb11ec0a3746bed9e2eaf5f84a Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sun, 15 Aug 2021 16:49:55 -0400 Subject: [PATCH 20/44] docs: Add information about waiting status to NEWS --- ChangeLog | 10 +++------- NEWS | 10 +++++++++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4c94acde6..6f260db33 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,12 +1,5 @@ 2.6.0 () - -- 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, @@ -92,6 +85,9 @@ Thanks to Scott Mcdermott - TW #2550 Write context skipped if description contains an identifier Thanks to Sebastian Fricke. +- TW #2554 Remove the waiting state, and consider any task with wait>now to be + waiting + Thanks to Dustin J. Mitchell ------ current release --------------------------- diff --git a/NEWS b/NEWS index ab900e217..26d81f202 100644 --- a/NEWS +++ b/NEWS @@ -27,7 +27,13 @@ New Features in Taskwarrior 2.6.0 'due.by:eod', which it would not otherwise. It also works with whole units like days, e.g. 'add test due:2021-07-17' would not match 'due.before:tomorrow' (on the 16th), but would match 'due.by:tomorrow'. - + - Waiting is now an entirely "virtual" concept, based on a task's + 'wait' property and the current time. Task is consiered "waiting" if its + wait attribute is in the future. TaskWarrior no longer explicitly + "unwaits" a task (the wait attribute is not removed once its value is in + the past), so the "unwait' verbosity token is no longer available. + This allows for filtering for tasks that were waiting in the past + intervals, but are not waiting anymore. New Commands in Taskwarrior 2.6.0 @@ -46,6 +52,8 @@ New Configuration Options in Taskwarrior 2.6.0 Newly Deprecated Features in Taskwarrior 2.6.0 - The 'PARENT' and 'CHILD' virtual tags are replaced by 'TEMPLATE' and 'INSTANCE'. + - The 'waiting' status is now deprecated. We recommend using +WAITING virtual tag + or wait-attribute based filters, such as 'wait.before:eow' instead. Fixed regressions in 2.6.0 From 413b8d22b7b4134c080e1a8eb7c965c60d84825e Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 15 Aug 2021 17:18:47 +0000 Subject: [PATCH 21/44] Remove references to the 'depends' property outside of Task.cpp With the exception of `taskDifferences` and `taskInfoDifferences`, deferred to #2572. --- src/TDB2.cpp | 33 ++++++++++++------------- src/Task.cpp | 50 ++++++++++++++++++++++++++++++-------- src/Task.h | 2 ++ src/columns/ColDepends.cpp | 25 ++++++++++--------- src/commands/CmdEdit.cpp | 3 ++- src/commands/CmdInfo.cpp | 4 +-- src/commands/CmdPurge.cpp | 3 +-- src/dependency.cpp | 38 +++-------------------------- src/feedback.cpp | 5 ++-- src/main.h | 2 -- src/sort.cpp | 19 +++++++++------ 11 files changed, 92 insertions(+), 92 deletions(-) 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; From 20af583e21666d4825bfb81fcd1264c786bf4d01 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 15 Aug 2021 20:18:02 +0000 Subject: [PATCH 22/44] 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 | 9 ++ doc/man/taskrc.5.in | 6 -- src/Context.cpp | 1 - src/Task.cpp | 201 ++++++++++++++++++++++----------------- src/Task.h | 6 +- src/commands/CmdInfo.cpp | 1 + src/commands/CmdShow.cpp | 1 - src/sort.cpp | 4 +- test/export.t | 14 --- test/import.t | 11 --- 10 files changed, 131 insertions(+), 123 deletions(-) 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) From 9768fb8bca970930dc42bfec5f5ac494512a9f7c Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Tue, 17 Aug 2021 20:52:12 -0400 Subject: [PATCH 23/44] docs: Sort ChangeLog entries --- ChangeLog | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 826455575..b47fcda75 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,14 +1,5 @@ 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, @@ -37,16 +28,18 @@ Thanks to Janik Rabe - TW #2017 Support 64-bit datetime values Thanks to Evgeniy Vasilev -- TW #2257 UDA string fields can't start with certain keywords - Thanks to Michael Russell. - TW #2060 Review timestamp is displayed as unix time, not formatted Thanks to JavaZauber - TW #2093 wrong order under projects command Thanks to Beka, Max Rossmannek. - TW #2101 Numeric UDA values above 2,147,483,647 overflow without warning Thanks to Adam Monsen. +- TW #2208 Feature: added coloring of dates with scheduled tasks to calendar + Thanks to Sebastian Uharek - TW #2247 Configuration override rc.verbose:off not respected Thanks to Vignesh Prabhu. +- TW #2257 UDA string fields can't start with certain keywords + Thanks to Michael Russell. - TW #2290 Support moving the config file to XDG_CONFIG_HOME Thanks to Julien Rabinow. - TW #2292 CmdEdit: Interruption should remove lock file @@ -80,8 +73,6 @@ Thanks to Scott Kostyshak. - TW #2503 Warn against executing an empty execute command. Thanks to heinrichat. -- TW #2208 Feature: added coloring of dates with scheduled tasks to calendar - Thanks to Sebastian Uharek - TW #2514 Duration values can be mis-reported in the task info output Thanks to reportaman. - TW #2519 Named date eod should be last minute of today and not first of @@ -97,6 +88,9 @@ - TW #2554 Remove the waiting state, and consider any task with wait>now to be waiting Thanks to Dustin J. Mitchell +- TW #2569 The `json.depends.array` configuration option is now ignored. + Dependencies are always represented as an array in JSON output. + Thanks to Dustin J. Mitchell ------ current release --------------------------- From e2e184b8d4e5f59d09810f492613566cf22492f3 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 00:45:12 -0400 Subject: [PATCH 24/44] CmdConfig: Be more strict when matching confiuration variables Allow only leading spaces in front of configuration variables, as opposed to arbitrary strings. This prevents matching variables like "report.list.context=" when one is seeking to modify "context=". --- src/commands/CmdConfig.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/commands/CmdConfig.cpp b/src/commands/CmdConfig.cpp index c4270115b..03fc6e6ba 100644 --- a/src/commands/CmdConfig.cpp +++ b/src/commands/CmdConfig.cpp @@ -64,23 +64,27 @@ bool CmdConfig::setConfigVariable ( for (auto& line : contents) { + // Get l-trimmed version of the line + auto trimmed_line = trim (line, " "); + // If there is a comment on the line, it must follow the pattern. auto comment = line.find ('#'); - auto pos = line.find (name + '='); + auto pos = trimmed_line.find (name + '='); - if (pos != std::string::npos && - (comment == std::string::npos || - comment > pos)) + // TODO: Use std::regex here + if (pos == 0) { found = true; if (!confirmation || confirm (format ("Are you sure you want to change the value of '{1}' from '{2}' to '{3}'?", name, Context::getContext ().config.get (name), value))) { - line = name + '=' + json::encode (value); + auto new_line = line.substr (0, pos + name.length () + 1) + json::encode (value); if (comment != std::string::npos) line += ' ' + line.substr (comment); + // Rewrite the line + line = new_line; change = true; } } @@ -115,13 +119,13 @@ int CmdConfig::unsetConfigVariable (const std::string& name, bool confirmation / { auto lineDeleted = false; - // If there is a comment on the line, it must follow the pattern. - auto comment = line->find ('#'); - auto pos = line->find (name + '='); + // Get l-trimmed version of the line - if (pos != std::string::npos && - (comment == std::string::npos || - comment > pos)) + // If there is a comment on the line, it must follow the pattern. + auto pos = trim (*line, " ").find (name + '='); + + // TODO: Use std::regex here + if (pos == 0) { found = true; From a4643246175598e51ca1f16596c971e46eb84c5e Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 00:46:36 -0400 Subject: [PATCH 25/44] CmdConfig: Properly preserve comments at the end of the line Trailing comments previously caused crash of the application. Closes #2581. --- src/commands/CmdConfig.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/CmdConfig.cpp b/src/commands/CmdConfig.cpp index 03fc6e6ba..6813ffd1f 100644 --- a/src/commands/CmdConfig.cpp +++ b/src/commands/CmdConfig.cpp @@ -80,8 +80,9 @@ bool CmdConfig::setConfigVariable ( { auto new_line = line.substr (0, pos + name.length () + 1) + json::encode (value); + // Preserve the comment if (comment != std::string::npos) - line += ' ' + line.substr (comment); + new_line += " " + line.substr (comment); // Rewrite the line line = new_line; From 1f768565793d99fc46c00fa666acd1392eaba6e0 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 01:01:19 -0400 Subject: [PATCH 26/44] tests: Add test for TW #2581 --- test/tw-2581.t | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100755 test/tw-2581.t diff --git a/test/tw-2581.t b/test/tw-2581.t new file mode 100755 index 000000000..b7120a3a6 --- /dev/null +++ b/test/tw-2581.t @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Test setting configuration variable with a trailing comment works +. bash_tap_tw.sh + +# Add configuration variable with a trailing comment into taskrc +echo 'weekstart=Monday # Europe standard' >> taskrc +cat taskrc + +# Use config the change the value to "Sunday" +task config weekstart Sunday + +# Ensure the comment was preserved and value changed +cat taskrc | grep weekstart=Sunday +[[ `cat taskrc | grep weekstart=Sunday` == 'weekstart=Sunday # Europe standard' ]] From c195c594946ab62a297dbe971d6e3a8f24272c71 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 01:03:07 -0400 Subject: [PATCH 27/44] docs: Document TW #2581 --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index b47fcda75..bed9aaa83 100644 --- a/ChangeLog +++ b/ChangeLog @@ -91,6 +91,7 @@ - TW #2569 The `json.depends.array` configuration option is now ignored. Dependencies are always represented as an array in JSON output. Thanks to Dustin J. Mitchell +- TW #2581 Config entry with a trailing comment cannot be modified ------ current release --------------------------- From ab29ef8326fbf747d1f5839fb286bb27bb44fafe Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Fri, 20 Aug 2021 20:17:50 -0400 Subject: [PATCH 28/44] CmdCustom: Respect report..context configuration variable This allows the user to configure if a specific report should (or should not) adhere to the currently active context. Closes #2560. --- src/commands/CmdCustom.cpp | 15 +++++++++++++++ src/commands/CmdCustom.h | 1 + src/commands/Command.h | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/commands/CmdCustom.cpp b/src/commands/CmdCustom.cpp index 1bfd8ca56..6fb338110 100644 --- a/src/commands/CmdCustom.cpp +++ b/src/commands/CmdCustom.cpp @@ -59,6 +59,21 @@ CmdCustom::CmdCustom ( _category = Category::report; } +//////////////////////////////////////////////////////////////////////////////// +// Whether a report uses context is defined by the report..context +// configuration variable. +// +bool CmdCustom::uses_context () const +{ + auto config = Context::getContext ().config; + auto key = "report." + _keyword + ".context"; + + if (config.has (key)) + return config.getBoolean (key); + else + return _uses_context; +} + //////////////////////////////////////////////////////////////////////////////// int CmdCustom::execute (std::string& output) { diff --git a/src/commands/CmdCustom.h b/src/commands/CmdCustom.h index 4b1dbf381..f88bc0303 100644 --- a/src/commands/CmdCustom.h +++ b/src/commands/CmdCustom.h @@ -34,6 +34,7 @@ class CmdCustom : public Command { public: CmdCustom (const std::string&, const std::string&, const std::string&); + bool uses_context () const override; int execute (std::string&); private: diff --git a/src/commands/Command.h b/src/commands/Command.h index 71c13bd30..f144a1cc3 100644 --- a/src/commands/Command.h +++ b/src/commands/Command.h @@ -63,7 +63,7 @@ public: bool read_only () const; bool displays_id () const; bool needs_gc () const; - bool uses_context () const; + virtual bool uses_context () const; bool accepts_filter () const; bool accepts_modifications () const; bool accepts_miscellaneous () const; From 5309132add2b4bd064d7033199efac98ef597f00 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Fri, 20 Aug 2021 20:51:44 -0400 Subject: [PATCH 29/44] CmdTimesheet: Add ability to specify if context should be used This makes timesheet consistent with other report commands. --- src/commands/CmdTimesheet.cpp | 16 ++++++++++++++++ src/commands/CmdTimesheet.h | 1 + 2 files changed, 17 insertions(+) diff --git a/src/commands/CmdTimesheet.cpp b/src/commands/CmdTimesheet.cpp index 307dc363c..4ec8ba406 100644 --- a/src/commands/CmdTimesheet.cpp +++ b/src/commands/CmdTimesheet.cpp @@ -53,6 +53,22 @@ CmdTimesheet::CmdTimesheet () _category = Command::Category::report; } +//////////////////////////////////////////////////////////////////////////////// +// Whether a the timesheet uses context is defined by the +// report.timesheet.context configuration variable. +// +bool CmdTimesheet::uses_context () const +{ + auto config = Context::getContext ().config; + auto key = "report.timesheet.context"; + + if (config.has (key)) + return config.getBoolean (key); + else + return _uses_context; +} + + //////////////////////////////////////////////////////////////////////////////// int CmdTimesheet::execute (std::string& output) { diff --git a/src/commands/CmdTimesheet.h b/src/commands/CmdTimesheet.h index 88fcc8326..dc6236bf1 100644 --- a/src/commands/CmdTimesheet.h +++ b/src/commands/CmdTimesheet.h @@ -35,6 +35,7 @@ class CmdTimesheet : public Command public: CmdTimesheet (); int execute (std::string&); + bool uses_context () const override; }; #endif From 21f1086f3def65a4b6008ce8ed0a0685d5680204 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Fri, 20 Aug 2021 20:54:08 -0400 Subject: [PATCH 30/44] Context: Add default values for report..context variables --- src/Context.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Context.cpp b/src/Context.cpp index b0499bb9b..408d4617c 100644 --- a/src/Context.cpp +++ b/src/Context.cpp @@ -293,103 +293,121 @@ std::string configurationDefaults = "report.long.columns=id,start.active,entry,modified.age,depends,priority,project,tags,recur,wait.remaining,scheduled,due,until,description\n" "report.long.filter=status:pending -WAITING\n" "report.long.sort=modified-\n" + "report.long.context=1\n" "\n" "report.list.description=Most details of tasks\n" "report.list.labels=ID,Active,Age,D,P,Project,Tags,R,Sch,Due,Until,Description,Urg\n" "report.list.columns=id,start.age,entry.age,depends.indicator,priority,project,tags,recur.indicator,scheduled.countdown,due,until.remaining,description.count,urgency\n" "report.list.filter=status:pending -WAITING\n" "report.list.sort=start-,due+,project+,urgency-\n" + "report.list.context=1\n" "\n" "report.ls.description=Few details of tasks\n" "report.ls.labels=ID,A,D,Project,Tags,R,Wait,S,Due,Until,Description\n" "report.ls.columns=id,start.active,depends.indicator,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due.countdown,until.countdown,description.count\n" "report.ls.filter=status:pending -WAITING\n" "report.ls.sort=start-,description+\n" + "report.ls.context=1\n" "\n" "report.minimal.description=Minimal details of tasks\n" "report.minimal.labels=ID,Project,Tags,Description\n" "report.minimal.columns=id,project,tags.count,description.count\n" "report.minimal.filter=status:pending\n" "report.minimal.sort=project+/,description+\n" + "report.minimal.context=1\n" "\n" "report.newest.description=Newest tasks\n" "report.newest.labels=ID,Active,Created,Age,Mod,D,P,Project,Tags,R,Wait,Sch,Due,Until,Description\n" "report.newest.columns=id,start.age,entry,entry.age,modified.age,depends.indicator,priority,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due,until.age,description\n" "report.newest.filter=status:pending\n" "report.newest.sort=entry-\n" + "report.newest.context=1\n" "\n" "report.oldest.description=Oldest tasks\n" "report.oldest.labels=ID,Active,Created,Age,Mod,D,P,Project,Tags,R,Wait,Sch,Due,Until,Description\n" "report.oldest.columns=id,start.age,entry,entry.age,modified.age,depends.indicator,priority,project,tags,recur.indicator,wait.remaining,scheduled.countdown,due,until.age,description\n" "report.oldest.filter=status:pending\n" "report.oldest.sort=entry+\n" + "report.oldest.context=1\n" "\n" "report.overdue.description=Overdue tasks\n" "report.overdue.labels=ID,Active,Age,Deps,P,Project,Tag,R,S,Due,Until,Description,Urg\n" "report.overdue.columns=id,start.age,entry.age,depends,priority,project,tags,recur.indicator,scheduled.countdown,due,until,description,urgency\n" "report.overdue.filter=status:pending and +OVERDUE\n" "report.overdue.sort=urgency-,due+\n" + "report.overdue.context=1\n" "\n" "report.active.description=Active tasks\n" "report.active.labels=ID,Started,Active,Age,D,P,Project,Tags,Recur,W,Sch,Due,Until,Description\n" "report.active.columns=id,start,start.age,entry.age,depends.indicator,priority,project,tags,recur,wait,scheduled.remaining,due,until,description\n" "report.active.filter=status:pending and +ACTIVE\n" "report.active.sort=project+,start+\n" + "report.active.context=1\n" "\n" "report.completed.description=Completed tasks\n" "report.completed.labels=ID,UUID,Created,Completed,Age,Deps,P,Project,Tags,R,Due,Description\n" "report.completed.columns=id,uuid.short,entry,end,entry.age,depends,priority,project,tags,recur.indicator,due,description\n" "report.completed.filter=status:completed\n" "report.completed.sort=end+\n" + "report.completed.context=1\n" "\n" "report.recurring.description=Recurring Tasks\n" "report.recurring.labels=ID,Active,Age,D,P,Project,Tags,Recur,Sch,Due,Until,Description,Urg\n" "report.recurring.columns=id,start.age,entry.age,depends.indicator,priority,project,tags,recur,scheduled.countdown,due,until.remaining,description,urgency\n" "report.recurring.filter=status:pending and (+PARENT or +CHILD)\n" "report.recurring.sort=due+,urgency-,entry+\n" + "report.recurring.context=1\n" "\n" "report.waiting.description=Waiting (hidden) tasks\n" "report.waiting.labels=ID,A,Age,D,P,Project,Tags,R,Wait,Remaining,Sched,Due,Until,Description\n" "report.waiting.columns=id,start.active,entry.age,depends.indicator,priority,project,tags,recur.indicator,wait,wait.remaining,scheduled,due,until,description\n" "report.waiting.filter=+WAITING\n" "report.waiting.sort=due+,wait+,entry+\n" + "report.waiting.context=1\n" "\n" "report.all.description=All tasks\n" "report.all.labels=ID,St,UUID,A,Age,Done,D,P,Project,Tags,R,Wait,Sch,Due,Until,Description\n" "report.all.columns=id,status.short,uuid.short,start.active,entry.age,end.age,depends.indicator,priority,project.parent,tags.count,recur.indicator,wait.remaining,scheduled.remaining,due,until.remaining,description\n" "report.all.sort=entry-\n" + "report.all.context=1\n" "\n" "report.next.description=Most urgent tasks\n" "report.next.labels=ID,Active,Age,Deps,P,Project,Tag,Recur,S,Due,Until,Description,Urg\n" "report.next.columns=id,start.age,entry.age,depends,priority,project,tags,recur,scheduled.countdown,due.relative,until.remaining,description,urgency\n" "report.next.filter=status:pending -WAITING limit:page\n" "report.next.sort=urgency-\n" + "report.next.context=1\n" "\n" "report.ready.description=Most urgent actionable tasks\n" "report.ready.labels=ID,Active,Age,D,P,Project,Tags,R,S,Due,Until,Description,Urg\n" "report.ready.columns=id,start.age,entry.age,depends.indicator,priority,project,tags,recur.indicator,scheduled.countdown,due.countdown,until.remaining,description,urgency\n" "report.ready.filter=+READY\n" "report.ready.sort=start-,urgency-\n" + "report.ready.context=1\n" "\n" "report.blocked.description=Blocked tasks\n" "report.blocked.columns=id,depends,project,priority,due,start.active,entry.age,description\n" "report.blocked.labels=ID,Deps,Proj,Pri,Due,Active,Age,Description\n" "report.blocked.sort=due+,priority-,start-,project+\n" "report.blocked.filter=status:pending -WAITING +BLOCKED\n" + "report.blocked.context=1\n" "\n" "report.unblocked.description=Unblocked tasks\n" "report.unblocked.columns=id,depends,project,priority,due,start.active,entry.age,description\n" "report.unblocked.labels=ID,Deps,Proj,Pri,Due,Active,Age,Description\n" "report.unblocked.sort=due+,priority-,start-,project+\n" "report.unblocked.filter=status:pending -WAITING -BLOCKED\n" + "report.unblocked.context=1\n" "\n" "report.blocking.description=Blocking tasks\n" "report.blocking.labels=ID,UUID,A,Deps,Project,Tags,R,W,Sch,Due,Until,Description,Urg\n" "report.blocking.columns=id,uuid.short,start.active,depends,project,tags,recur,wait,scheduled.remaining,due.relative,until.remaining,description.count,urgency\n" "report.blocking.sort=urgency-,due+,entry+\n" "report.blocking.filter=status:pending -WAITING +BLOCKING\n" + "report.blocking.context=1\n" "\n" "report.timesheet.filter=(+PENDING and start.after:now-4wks) or (+COMPLETED and end.after:now-4wks)\n" + "report.timesheet.context=0\n" "\n"; // Supported modifiers, synonyms on the same line. From 8cad6487c794ca1309a3c89f6746b3627fb0ae8d Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 00:15:03 -0400 Subject: [PATCH 31/44] CLI2: Call uses_context from child classes, if applicable --- src/CLI2.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/CLI2.cpp b/src/CLI2.cpp index 5d9ce3666..2240d026e 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -34,6 +34,8 @@ #include #include #include +#include +#include // Overridden by rc.abbreviation.minimum. int CLI2::minimumMatchLength = 3; @@ -945,7 +947,23 @@ void CLI2::categorizeArgs () // Context is only applied for commands that request it. std::string command = getCommand (); Command* cmd = Context::getContext ().commands[command]; - if (cmd && cmd->uses_context ()) + + // Determine if the command uses Context. CmdCustom and CmdTimesheet need to + // be handled separately, as they override the parent Command::use_context + // method, and this is a pointer to Command class. + // + // All Command classes overriding uses_context () getter need to be specified + // here. + bool uses_context; + if (dynamic_cast (cmd)) + uses_context = (dynamic_cast (cmd))->uses_context (); + else if (dynamic_cast (cmd)) + uses_context = (dynamic_cast (cmd))->uses_context (); + else if (cmd) + uses_context = cmd->uses_context (); + + // Apply the context, if applicable + if (cmd && uses_context) addContext (cmd->accepts_filter (), cmd->accepts_modifications ()); bool changes = false; From 48bf64a501690129506ea35220b272ece73b3c36 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 00:16:04 -0400 Subject: [PATCH 32/44] tests: Add test for report..context variable --- test/context.t | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/context.t b/test/context.t index a213b93a5..2266a40b9 100755 --- a/test/context.t +++ b/test/context.t @@ -490,6 +490,33 @@ class ContextEvaluationTest(TestCase): self.assertNotIn("work today task", output) self.assertNotIn("home today task", output) + def test_context_ignored(self): + """Test the context is not applied with report list command if + report.list.context is set to 0.""" + + # Turn off context for this report + self.t.config("report.list.context", "0") + + # Get the tasks + code, out, err = self.t('list') + + # Assert all the tasks are present in the output + self.assertIn("work task", out) + self.assertIn("home task", out) + self.assertIn("work today task", out) + self.assertIn("home today task", out) + + # Set the home context and rerun the report + self.t('context home') + + code, out, err = self.t('list') + + # Assert nothing changed - all the tasks are present in the output + self.assertIn("work task", out) + self.assertIn("home task", out) + self.assertIn("work today task", out) + self.assertIn("home today task", out) + class ContextErrorHandling(TestCase): def setUp(self): From 05904549a0693278dba40fccf00043734857e1fc Mon Sep 17 00:00:00 2001 From: Bharatvaj H Date: Sat, 21 Aug 2021 06:24:12 +0530 Subject: [PATCH 33/44] Fixes #2580 Check annotations field before parsing Add test case for annotations --- src/Task.cpp | 5 +++++ test/import.t | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/Task.cpp b/src/Task.cpp index cc0316c9d..89f7de6ce 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -793,6 +793,11 @@ void Task::parseJSON (const json::object* root_obj) { std::map annos; + // Fail if 'annotations' is not an array + if (i.second->type() != json::j_array) { + throw format ("Annotations is malformed: {1}", i.second->dump ()); + } + auto atts = (json::array*)i.second; for (auto& annotations : atts->_data) { diff --git a/test/import.t b/test/import.t index eb5b48f42..638db6ed5 100755 --- a/test/import.t +++ b/test/import.t @@ -292,6 +292,12 @@ class TestImportValidate(TestCase): code, out, err = self.t.runError("import", input=j) self.assertIn("The status 'foo' is not valid.", err) + def test_import_malformed_annotation(self): + """Verify invalid 'annnotations' is caught""" + j = '{"description": "bad", "annotations": "bad"}' + code, out, err = self.t.runError("import", input=j) + self.assertIn('Annotations is malformed: "bad"', err) + class TestImportWithoutISO(TestCase): def setUp(self): From 4f14c529b02958530d84291bee6b260969a72d21 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 09:32:03 -0400 Subject: [PATCH 34/44] tests: Add test for TW #2577 This is already fixed on 2.6.0 thanks to #2405, but it never hurts to have more tests. Closes #2577. --- test/filter.t | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/filter.t b/test/filter.t index 7bc12c41b..f06999058 100755 --- a/test/filter.t +++ b/test/filter.t @@ -1130,6 +1130,24 @@ class TestBug1915(TestCase): self.assertIn("thingB", out) self.assertNotIn("thingC", out) + +class Test2577(TestCase): + def setUp(self): + self.t = Task() + + def test_filtering_for_datetime_like(self): + """2577: Check that filtering for datetime-like project names works""" + self.t('add one pro:sat') # looks like "saturday" + self.t('add two pro:whatever') + + # This should not fail (fails on 2.5.3) + code, out, err = self.t('pro:sat') + + # Assert expected output, but the crucial part of this test is success + # of the call above + self.assertIn("one", out) + + if __name__ == "__main__": from simpletap import TAPTestRunner unittest.main(testRunner=TAPTestRunner()) From 3471e1cdaa726edd7d8c4ca267036c2ad8ea2d81 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 09:57:02 -0400 Subject: [PATCH 35/44] docs: Add bharatvaj among AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index dcf92ac16..309e51e53 100644 --- a/AUTHORS +++ b/AUTHORS @@ -157,6 +157,7 @@ The following submitted code, packages or analysis, and deserve special thanks: Julien Rabinow Daniel Mowitz Scott Mcdermott + Bharatvaj Thanks to the following, who submitted detailed bug reports and excellent suggestions: From 55f38bd48e807f57a6018e3c67967deffe99c04e Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 10:01:26 -0400 Subject: [PATCH 36/44] docs: Document TW #2580 --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index bed9aaa83..fd9971318 100644 --- a/ChangeLog +++ b/ChangeLog @@ -91,6 +91,8 @@ - TW #2569 The `json.depends.array` configuration option is now ignored. Dependencies are always represented as an array in JSON output. Thanks to Dustin J. Mitchell +- TW #2580 Importing malformed JSON task crashes TW + Thanks to bharatvaj. - TW #2581 Config entry with a trailing comment cannot be modified ------ current release --------------------------- From 08dab41d488d23962e9d908a2f682b76c576b4d8 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 10:09:45 -0400 Subject: [PATCH 37/44] docs: Document TW #2560 --- ChangeLog | 2 ++ NEWS | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index fd9971318..f216b4ce9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -88,6 +88,8 @@ - TW #2554 Remove the waiting state, and consider any task with wait>now to be waiting Thanks to Dustin J. Mitchell +- TW #2560 Add report..context configuration variable + Thanks to Jake C. - TW #2569 The `json.depends.array` configuration option is now ignored. Dependencies are always represented as an array in JSON output. Thanks to Dustin J. Mitchell diff --git a/NEWS b/NEWS index 26d81f202..825900a5e 100644 --- a/NEWS +++ b/NEWS @@ -44,6 +44,10 @@ New Configuration Options in Taskwarrior 2.6.0 - The context definitions for reporting commmands are now stored in "context..read". Context definitions for write commands are now supported using "context..write" configuration variable. + - Each report (and the timesheet command) can explicitly opt-out from the + currently active context by setting the report..context variable to 0 + (defaults to 1). Useful for defining universal reports that ignore + currently set context, such as 'inbox' report for GTD methodology. - Multi-day holidays are now supported. Use holiday..start= and holiday..end= to specify a range-based holiday, such as a vacation. From d515326fbaf069dc975524b6db4dffd55e0c219f Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 10:09:59 -0400 Subject: [PATCH 38/44] docs: Add Jake C. among AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 309e51e53..4805776a7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -345,3 +345,4 @@ suggestions: Arvedui reportaman Pablo Vizcay + Jake C. From d270ef31a4b940f96a4e71d2243ea15731d67015 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 21 Aug 2021 12:39:17 -0400 Subject: [PATCH 39/44] docs: Document report.X.context in the man page --- doc/man/taskrc.5.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/man/taskrc.5.in b/doc/man/taskrc.5.in index b8c14c70e..69d1502f5 100644 --- a/doc/man/taskrc.5.in +++ b/doc/man/taskrc.5.in @@ -1279,6 +1279,12 @@ The description for report X when running the "task help" command. This is a comma-separated list of columns and formatting specifiers. See the command 'task columns' for a full list of options and examples. +.TP +.B report.X.context +A boolean value representing whether the given report should respect (apply) +the currently active context. See CONTEXT section for details about context. +Defaults to 1. + .TP .B report.X.labels The labels for each column that will be used when generating report X. The From 0523ada9fc055f7eb9edbc3d828b72b96988396e Mon Sep 17 00:00:00 2001 From: sebu06 Date: Sat, 28 Aug 2021 10:10:47 +0200 Subject: [PATCH 40/44] Fixed bug with double escaped single quotes Before, the parser always escaped single quotes, independent of the quotes being escaped already. This is now fixed. --- ChangeLog | 2 ++ src/CLI2.cpp | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f216b4ce9..c6a05a034 100644 --- a/ChangeLog +++ b/ChangeLog @@ -22,6 +22,8 @@ Thanks to Florian. - TW #1955 Adding tasks in context. Thanks to Jean-Francois Joly, Matt Smith. +- TW #1960 Fixed bug with double escaped single quotes. + Thanks to Sebastian Uharek - TW #2004 "shell" should not be expand to "exec tasksh" Thanks to Arvedui - TW #2007 Compute number of current tasks correctly diff --git a/src/CLI2.cpp b/src/CLI2.cpp index 2240d026e..cf6ddd61f 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -417,8 +417,18 @@ void CLI2::lexArguments () else { std::string quote = "'"; - std::string escaped = _original_args[i].attribute ("raw"); - escaped = str_replace (escaped, quote, "\\'"); + // Escape unescaped single quotes + std::string escaped = ""; + bool nextEscaped = false; + for(auto c : _original_args[i].attribute ("raw")) { + if(!nextEscaped && (c == '\\')) { + nextEscaped = true; + } else { + if(c == '\'' && !nextEscaped) escaped += "\\"; + nextEscaped = false; + } + escaped += c; + } std::string::size_type cursor = 0; std::string word; From c119b6d1deddf4595ab6365e3782e7efcd4d8679 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 28 Aug 2021 10:18:24 -0400 Subject: [PATCH 41/44] CLI2: Support escaped utf-8 characters --- src/CLI2.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/CLI2.cpp b/src/CLI2.cpp index cf6ddd61f..ae50e0400 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -36,6 +36,8 @@ #include #include #include +#include + // Overridden by rc.abbreviation.minimum. int CLI2::minimumMatchLength = 3; @@ -413,24 +415,31 @@ void CLI2::lexArguments () _args.push_back (a); } - // Process muktiple-token arguments. + // Process multiple-token arguments. else { std::string quote = "'"; + // Escape unescaped single quotes std::string escaped = ""; + std::string::size_type cursor = 0; + bool nextEscaped = false; - for(auto c : _original_args[i].attribute ("raw")) { - if(!nextEscaped && (c == '\\')) { + + while (int num = utf8_next_char (_original_args[i].attribute ("raw"), cursor)) + { + std::string character = utf8_character (num); + if (!nextEscaped && (character == "\\")) nextEscaped = true; - } else { - if(c == '\'' && !nextEscaped) escaped += "\\"; + else { + if (character == "\'" && !nextEscaped) + escaped += "\\"; nextEscaped = false; } - escaped += c; + escaped += character; } - std::string::size_type cursor = 0; + cursor = 0; std::string word; if (Lexer::readWord (quote + escaped + quote, quote, cursor, word)) { From ddf2f122a2e2296276b53fec5fd0519c6be64a99 Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 28 Aug 2021 10:26:17 -0400 Subject: [PATCH 42/44] CLI2: Pre-reserve the size of the escaped string --- src/CLI2.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/CLI2.cpp b/src/CLI2.cpp index ae50e0400..8866179ee 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -422,10 +422,12 @@ void CLI2::lexArguments () // Escape unescaped single quotes std::string escaped = ""; + + // For performance reasons. The escaped string is as long as the original. + escaped.reserve (_original_args[i].attribute ("raw").size ()); + std::string::size_type cursor = 0; - bool nextEscaped = false; - while (int num = utf8_next_char (_original_args[i].attribute ("raw"), cursor)) { std::string character = utf8_character (num); From 2619435148089ad4da6c2c27828b0fc792d6b85e Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 28 Aug 2021 10:39:44 -0400 Subject: [PATCH 43/44] tests: Add test for TW-2189 --- test/tw-2189.t | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100755 test/tw-2189.t diff --git a/test/tw-2189.t b/test/tw-2189.t new file mode 100755 index 000000000..5a23e5583 --- /dev/null +++ b/test/tw-2189.t @@ -0,0 +1,17 @@ +#!/bin/bash +. bash_tap_tw.sh + +task add "foo \' bar" +task list + +# Assert the task was correctly added +[[ ! -z `task list | grep "foo ' bar"` ]] +[[ `task _get 1.description` == "foo ' bar" ]] + +# Bonus: Assert escaped double quotes are also handled correctly +task add 'foo \" bar' +task list + +# Assert the task was correctly added +[[ ! -z `task list | grep 'foo " bar'` ]] +[[ `task _get 2.description` == 'foo " bar' ]] From 8b30046d0a2486d1c61f7903cd06a0551e2cb4af Mon Sep 17 00:00:00 2001 From: Tomas Babej Date: Sat, 28 Aug 2021 20:28:32 -0400 Subject: [PATCH 44/44] CLI2: Simplify code by using const quote string The "\'" string is equal to "'", which is already stored in the quote variable, so we might as well use that. Thanks to Sebastian Uharek for the review suggestion. --- src/CLI2.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CLI2.cpp b/src/CLI2.cpp index 8866179ee..4a8e7fda7 100644 --- a/src/CLI2.cpp +++ b/src/CLI2.cpp @@ -418,7 +418,7 @@ void CLI2::lexArguments () // Process multiple-token arguments. else { - std::string quote = "'"; + const std::string quote = "'"; // Escape unescaped single quotes std::string escaped = ""; @@ -434,7 +434,7 @@ void CLI2::lexArguments () if (!nextEscaped && (character == "\\")) nextEscaped = true; else { - if (character == "\'" && !nextEscaped) + if (character == quote && !nextEscaped) escaped += "\\"; nextEscaped = false; }