Hooks

- Removed the ability for hooks to add tasks, or modify tasks that are outside
  the context of the current event. This makes hooks a local mechanism that
  operates only on local changes. Modifications/additions coming in via sync
  command are not processed by hooks.

ChangeLog

@@ -1,6 +1,6 @@
 2.4.1 () -
 
-- TW-1457 Non-existant attributes are not properly handled (thanks to Tomas
+- TW-1457 Non-existent attributes are not properly handled (thanks to Tomas
           Babej).
 - TW-1484 The 'history' and 'ghistory' reports do not obey rc.color.label.
 - TW-1486 task wait shows completed tasks which has a wait attribute (thanks to

scripts/hooks/on-add

@@ -1,20 +1,19 @@
 #!/bin/bash
 
-# The on-add event is triggered separately for each task added
+# The on-add event is triggered separately for each task added. This hook
+# script can accept/reject the addition. Processing will continue.
 
 # Input:
-# - line of JSON for the task added
-
-# Output:
-# - all emitted JSON lines are added/modified as tasks, if the exit code is
-#   zero, otherwise ignored.
-# - minimal new task:  {"description":"Buy milk"}
-# - to modify a task include complete JSON
-# - all emitted non-JSON lines are considered feedback messages if the exit
-#   code is zero, otherwise they are considered errors.
-
+# - Line of JSON for proposed new task.
 read new_task
 
+# Output:
+# - JSON, modified or unmodified.
+# - Optional feedback/error.
 echo $new_task
 echo 'on-add'
+
+# Status:
+# - 0:     JSON accepted, non-JSON is feedback.
+# - non-0: JSON ignored, non-JSON is error.
 exit 0

scripts/hooks/on-exit

@@ -1,20 +1,20 @@
 #!/bin/bash
 
-# The on-exit event is triggered once, after all processing is complete, i.e.
-# last
+# The on-exit event is triggered once, after all processing is complete.
+# This hooks script has no effect on processing.
 
 # Input:
-# - read-only line of JSON for each task added/modified
-
-# Output:
-# - any emitted JSON is ignored
-# - all emitted non-JSON lines are considered feedback messages if the exit
-#   code is zero, otherwise they are considered errors.
-
-while read -t 1 modified_task
+# - Read-only line of JSON for each task added/modified
+while read modified_task
 do
-  # Scan task
+  echo $modified_task
 done
 
+# Output:
+# - Optional feedback/error.
 echo 'on-exit'
+
+# Status:
+# - 0:     JSON ignored, non-JSON is feedback.
+# - non-0: JSON ignored, non-JSON is error.
 exit 0

scripts/hooks/on-launch

@@ -1,18 +1,17 @@
 #!/bin/bash
 
 # The on-launch event is triggered once, after initialization, before any
-# processing occurs, i.e first
+# processing occurs. This hooks script has no effect on processing.
 
 # Input:
-# - none
+# - None
 
 # Output:
-# - all emitted JSON lines are added/modified as tasks, if the exit code is
-#   zero, otherwise ignored.
-# - minimal new task:  {"description":"Buy milk"}
-# - to modify a task include complete JSON
-# - all emitted non-JSON lines are considered feedback messages if the exit
-#   code is zero, otherwise they are considered errors.
-
+# - Optional feedback/error.
 echo 'on-launch'
+
+# Status:
+# - 0:     JSON ignored, non-JSON is feedback.
+# - non-0: JSON ignored, non-JSON is error.
 exit 0
+

scripts/hooks/on-modify

@@ -1,22 +1,21 @@
 #!/bin/bash
 
-# The on-modify event is triggered separately for each task added or modified
+# The on-modify event is triggered separately for each task modified. This hook
+# script can accept/reject the modification. Processing will continue.
 
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-
-# Output:
-# - all emitted JSON lines are added/modified as tasks, if the exit code is
-#   zero, otherwise ignored.
-# - minimal new task:  {"description":"Buy milk"}
-# - to modify a task include complete JSON
-# - all emitted non-JSON lines are considered feedback messages if the exit
-#   code is zero, otherwise they are considered errors.
-
 read original_task
 read modified_task
 
+# Output:
+# - JSON, modified or unmodified.
+# - Optional feedback/error.
 echo $modified_task
 echo 'on-modify'
+
+# Status:
+# - 0:     JSON accepted, non-JSON is feedback.
+# - non-0: JSON ignored, non-JSON is error.
 exit 0

src/Hooks.cpp

@@ -109,8 +109,7 @@ bool Hooks::enable (bool value)
 // - none
 //
 // Output:
-// - all emitted JSON lines are added/modified as tasks, if the exit code is
-//   zero, otherwise ignored.
+// - all emitted JSON is ignored.
 // - all emitted non-JSON lines are considered feedback or error messages
 //   depending on the status code.
 //
@@ -136,12 +135,11 @@ void Hooks::onLaunch ()
       {
         if (isJSON (*line))
         {
-          if (status == 0)
-          {
-            // Only 'add' is possible.
-            Task newTask (*line);
-            context.tdb2.add (newTask);
-          }
+          if (_debug >= 2)
+            context.error ("Line of JSON output ignored: " + (*line));
+
+          else if (_debug >= 1)
+            context.error ("Line(s) of JSON output ignored.");
         }
         else
         {
@@ -153,7 +151,10 @@ void Hooks::onLaunch ()
       }
 
       if (status)
+      {
+        // TODO Hooks debug info needed.
         throw 0;  // This is how hooks silently terminate processing.
+      }
     }
   }
 
@@ -168,7 +169,7 @@ void Hooks::onLaunch ()
 // - read-only line of JSON for each task added/modified
 //
 // Output:
-// - any emitted JSON is ignored
+// - all emitted JSON is ignored
 // - all emitted non-JSON lines are considered feedback or error messages
 //   depending on the status code.
 //
@@ -220,7 +221,10 @@ void Hooks::onExit ()
       }
 
       if (status)
+      {
+        // TODO Hooks debug info needed.
         throw 0;  // This is how hooks silently terminate processing.
+      }
     }
   }
 
@@ -234,14 +238,14 @@ void Hooks::onExit ()
 // - line of JSON for the task added
 //
 // Output:
-// - all emitted JSON lines are added/modified as tasks, if the exit code is
-//   zero, otherwise ignored.
+// - emitted JSON for the input task is added, if the exit code is zero,
+//   otherwise ignored.
 // - all emitted non-JSON lines are considered feedback or error messages
 //   depending on the status code.
 //
-void Hooks::onAdd (std::vector <Task>& tasks)
+void Hooks::onAdd (Task& task)
 {
-  if (! _enabled || tasks.size () < 1)
+  if (! _enabled)
     return;
 
   context.timer_hooks.start ();
@@ -251,7 +255,7 @@ void Hooks::onAdd (std::vector <Task>& tasks)
   {
     // Convert vector of tasks to a vector of strings.
     std::vector <std::string> input;
-    input.push_back (tasks[0].composeJSON ());
+    input.push_back (task.composeJSON ());
 
     // Call the hook scripts.
     std::vector <std::string>::iterator script;
@@ -260,14 +264,18 @@ void Hooks::onAdd (std::vector <Task>& tasks)
       std::vector <std::string> output;
       int status = callHookScript (*script, input, output);
 
-      input.clear ();
       std::vector <std::string>::iterator line;
       for (line = output.begin (); line != output.end (); ++line)
       {
         if (isJSON (*line))
         {
+          // TODO Verify the same task UUID.
+          // TODO Verify only one task.
+
           if (status == 0)
-            input.push_back (*line);
+            input[0] = *line;
+          else
+            ; // TODO Hooks debug info needed.
         }
         else
         {
@@ -279,14 +287,14 @@ void Hooks::onAdd (std::vector <Task>& tasks)
       }
 
       if (status)
+      {
+        // TODO Hooks debug info needed.
         throw 0;  // This is how hooks silently terminate processing.
+      }
     }
 
-    // Transfer the modified task lines back to the original task list.
-    tasks.clear ();
-    std::vector <std::string>::iterator i;
-    for (i = input.begin (); i != input.end (); ++i)
-      tasks.push_back (Task (*i));
+    // Transfer the modified task back to the original task.
+    task = Task (input[0]);
   }
 
   context.timer_hooks.stop ();
@@ -300,14 +308,14 @@ void Hooks::onAdd (std::vector <Task>& tasks)
 // - line of JSON for the modified task, the diff being the modification
 //
 // Output:
-// - all emitted JSON lines are added/modified as tasks, if the exit code is
-//   zero, otherwise ignored.
+// - emitted JSON for the input task is saved, if the exit code is zero,
+//   otherwise ignored.
 // - all emitted non-JSON lines are considered feedback or error messages
 //   depending on the status code.
 //
-void Hooks::onModify (const Task& before, std::vector <Task>& tasks)
+void Hooks::onModify (const Task& before, Task& after)
 {
-  if (! _enabled || tasks.size () < 1)
+  if (! _enabled)
     return;
 
   context.timer_hooks.start ();
@@ -315,60 +323,49 @@ void Hooks::onModify (const Task& before, std::vector <Task>& tasks)
   std::vector <std::string> matchingScripts = scripts ("on-modify");
   if (matchingScripts.size ())
   {
-    // Prepare invariants.
-    std::string beforeJSON = before.composeJSON ();
-
     // Convert vector of tasks to a vector of strings.
     std::vector <std::string> input;
-    input.push_back (beforeJSON);               // [0] original, never changes
-    input.push_back (tasks[0].composeJSON ());  // [1] original'
+    input.push_back (before.composeJSON ()); // [line 0] original, never changes
+    input.push_back (after.composeJSON ());  // [line 1] modified
 
     // Call the hook scripts.
     std::vector <std::string>::iterator script;
     for (script = matchingScripts.begin (); script != matchingScripts.end (); ++script)
     {
-      std::vector <std::string> firstTwoOnly;
-      firstTwoOnly.push_back (input[0]);
-      firstTwoOnly.push_back (input[1]);
-
       std::vector <std::string> output;
-      int status = callHookScript (*script, firstTwoOnly, output);
-
-      // Start from scratch.
-      input[1] = "";                            // [1] placeholder for original'
+      int status = callHookScript (*script, input, output);
 
       std::vector <std::string>::iterator line;
       for (line = output.begin (); line != output.end (); ++line)
       {
         if (isJSON (*line))
         {
+          // TODO Verify the same task UUID.
+          // TODO Verify only one task.
+
           if (status == 0)
           {
             if (JSONContainsUUID ((*line), before.get ("uuid")))
               input[1] = *line;                 // [1] original'
             else
-              input.push_back (*line);          // [n > 1] extras
+              ; // TODO Error/debug
           }
         }
         else
         {
           if (status == 0)
-              context.footnote (*line);
-        else
+            context.footnote (*line);
+          else
             context.error (*line);
         }
       }
 
       if (status)
+      {
+        // TODO Hooks debug info needed.
         throw 0;  // This is how hooks silently terminate processing.
+      }
     }
-
-    // Transfer the modified task lines back to the original task list.
-    tasks.clear ();
-    std::vector <std::string>::iterator i;
-    for (i = input.begin (); i != input.end (); ++i)
-      if (i != input.begin ())
-        tasks.push_back (Task (*i));
   }
 
   context.timer_hooks.stop ();

src/Hooks.h

@@ -44,8 +44,8 @@ public:
 
   void onLaunch ();
   void onExit ();
-  void onAdd (std::vector <Task>&);
-  void onModify (const Task&, std::vector <Task>&);
+  void onAdd (Task&);
+  void onModify (const Task&, Task&);
 
   std::vector <std::string> list ();
 

src/TDB2.cpp

@@ -567,12 +567,12 @@ void TDB2::add (Task& task, bool add_to_backlog /* = true */)
   if (!verifyUniqueUUID (uuid))
     throw format (STRING_TDB2_UUID_NOT_UNIQUE, uuid);
 
-  // Create a vector tasks, as hooks can cause them to multiply.
-  std::vector <Task> changes;
-  changes.push_back (task);
-  context.hooks.onAdd (changes);
+  // Only locally-added tasks trigger hooks.  This means that tasks introduced
+  // via 'sync' do not trigger hooks.
+  if (add_to_backlog)
+    context.hooks.onAdd (task);
 
-  update (uuid, task, add_to_backlog, changes);
+  update (uuid, task, add_to_backlog);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -584,77 +584,64 @@ void TDB2::modify (Task& task, bool add_to_backlog /* = true */)
   std::string uuid = task.get ("uuid");
 
   // Get the unmodified task as reference, so the hook can compare.
-  Task original;
-  get (uuid, original);
+  if (add_to_backlog)
+  {
+    Task original;
+    get (uuid, original);
+    context.hooks.onModify (original, task);
+  }
 
-  // Create a vector tasks, as hooks can cause them to multiply.
-  std::vector <Task> changes;
-  changes.push_back (task);
-  context.hooks.onModify (original, changes);
-
-  update (uuid, task, add_to_backlog, changes);
+  update (uuid, task, add_to_backlog);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 void TDB2::update (
   const std::string& uuid,
   Task& task,
-  const bool add_to_backlog,
-  std::vector <Task>& changes)
+  const bool add_to_backlog)
 {
-  std::vector <Task>::iterator i;
-  for (i = changes.begin (); i != changes.end (); ++i)
+  // Validate to add metadata.
+  task.validate (false);
+
+  // If the task already exists, it is a modification, else addition.
+  Task original;
+  if (get (task.get ("uuid"), original))
   {
-    // Validate to add metadata.
-    i->validate (false);
+    // Update the task, wherever it is.
+    if (!pending.modify_task (task))
+      completed.modify_task (task);
 
-    // If the task already exists, it is a modification, else addition.
-    Task original;
-    if (get (i->get ("uuid"), original))
-    {
-      // Update the task, wherever it is.
-      if (!pending.modify_task (*i))
-        completed.modify_task (*i);
-
-      // time <time>
-      // old <task>
-      // new <task>
-      // ---
-      undo.add_line ("time " + Date ().toEpochString () + "\n");
-      undo.add_line ("old " + original.composeF4 () + "\n");
-      undo.add_line ("new " + i->composeF4 () + "\n");
-      undo.add_line ("---\n");
-    }
-    else
-    {
-      // Re-validate to add default values.
-      i->validate ();
-
-      // Add new task to either pending or completed.
-      std::string status = i->get ("status");
-      if (status == "completed" ||
-          status == "deleted")
-        completed.add_task (*i);
-      else
-        pending.add_task (*i);
-
-      // Add undo data lines:
-      //   time <time>
-      //   new <task>
-      //   ---
-      undo.add_line ("time " + Date ().toEpochString () + "\n");
-      undo.add_line ("new " + i->composeF4 () + "\n");
-      undo.add_line ("---\n");
-    }
-
-    // Add task to backlog.
-    if (add_to_backlog)
-      backlog.add_line (i->composeJSON () + "\n");
-
-    // The original task may be further referenced by the caller.
-    if (i->get ("uuid") == uuid)
-      task = *i;
+    // time <time>
+    // old <task>
+    // new <task>
+    // ---
+    undo.add_line ("time " + Date ().toEpochString () + "\n");
+    undo.add_line ("old " + original.composeF4 () + "\n");
+    undo.add_line ("new " + task.composeF4 () + "\n");
+    undo.add_line ("---\n");
   }
+  else
+  {
+    // Add new task to either pending or completed.
+    std::string status = task.get ("status");
+    if (status == "completed" ||
+        status == "deleted")
+      completed.add_task (task);
+    else
+      pending.add_task (task);
+
+    // Add undo data lines:
+    //   time <time>
+    //   new <task>
+    //   ---
+    undo.add_line ("time " + Date ().toEpochString () + "\n");
+    undo.add_line ("new " + task.composeF4 () + "\n");
+    undo.add_line ("---\n");
+  }
+
+  // Add task to backlog.
+  if (add_to_backlog)
+    backlog.add_line (task.composeJSON () + "\n");
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/TDB2.h

@@ -128,7 +128,7 @@ public:
 
 private:
   void gather_changes ();
-  void update (const std::string&, Task&, const bool, std::vector <Task>&);
+  void update (const std::string&, Task&, const bool);
   bool verifyUniqueUUID (const std::string&);
   void show_diff (const std::string&, const std::string&, const std::string&);
   void revert_undo (std::vector <std::string>&, std::string&, std::string&, std::string&, std::string&);