TW-1572: Implement better urgency inheritance

- Implement recursive urgency inheritance.  If this is to be made a
  default setting, some thought will have to be put into making it
  more efficient.

ChangeLog

@@ -1,5 +1,7 @@
 2.4.5 () -
 
+- TW-1572 Better urgency inheritance (thanks to Jens Erat).
+
 ------ current release ---------------------------
 
 2.4.4 (2015-05-10) df49aaba126484b668c41d3ff9301f8d8ec49987

doc/man/taskrc.5.in

@@ -1062,10 +1062,6 @@ Urgency coefficient for blocking tasks
 .RS
 Urgency coefficient for blocked tasks
 .RE
-.B urgency.inherit.coefficient=0.0
-.RS
-Urgency inherited from dependency chain
-.RE
 .B urgency.due.coefficient=12.0
 .RS
 Urgency coefficient for due dates
@@ -1127,6 +1123,16 @@ The coefficients reflect the relative importance of the various terms in the
 urgency calculation. These are default values, and may be modified to suit your
 preferences, but it is important that you carefully consider any modifications.
 
+.B urgency.inherit=off
+.RS
+Not actually a coefficient. When enabled, blocking tasks inherit
+the highest urgency value found in the tasks they block. This is
+done recursively.
+It is recommended to set urgency.blocking.coefficient and
+urgency.blocked.coefficient to 0.0 in order for this setting to
+be the most useful.
+.RE
+
 .SS DEFAULTS
 
 .TP

scripts/vim/syntax/taskrc.vim

@@ -151,7 +151,6 @@ syn match taskrcGoodKey '^\s*\Vurgency.annotations.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.blocked.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.blocking.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.due.coefficient='he=e-1
-syn match taskrcGoodKey '^\s*\Vurgency.inherit.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.next.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.uda.priority.H.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.uda.priority.M.coefficient='he=e-1
@@ -161,6 +160,7 @@ syn match taskrcGoodKey '^\s*\Vurgency.scheduled.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.tags.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.uda.\S\{-}.coefficient='he=e-1
 syn match taskrcGoodKey '^\s*\Vurgency.waiting.coefficient='he=e-1
+syn match taskrcGoodKey '^\s*\Vurgency.inherit='he=e-1
 syn match taskrcGoodKey '^\s*\Vverbose='he=e-1
 syn match taskrcGoodKey '^\s*\Vweekstart='he=e-1
 syn match taskrcGoodKey '^\s*\Vxterm.title='he=e-1

src/Config.cpp

@@ -154,8 +154,8 @@ std::string Config::_defaults =
   "urgency.tags.coefficient=1.0                   # Urgency coefficient for tags\n"
   "urgency.project.coefficient=1.0                # Urgency coefficient for projects\n"
   "urgency.blocked.coefficient=-5.0               # Urgency coefficient for blocked tasks\n"
-  "urgency.inherit.coefficient=0.0                # Urgency coefficient for blocked tasks inheriting from blocking tasks\n"
   "urgency.waiting.coefficient=-3.0               # Urgency coefficient for waiting status\n"
+  "urgency.inherit=off                            # Recursively inherit highest urgency value from blocked tasks\n"
   "urgency.age.max=365                            # Maximum age in days\n"
   "\n"
   "#urgency.user.project.foo.coefficient=5.0      # Urgency coefficients for 'foo' project\n"

src/Context.cpp

@@ -643,7 +643,6 @@ void Context::staticInitialization ()
   Task::urgencyScheduledCoefficient   = config.getReal ("urgency.scheduled.coefficient");
   Task::urgencyWaitingCoefficient     = config.getReal ("urgency.waiting.coefficient");
   Task::urgencyBlockedCoefficient     = config.getReal ("urgency.blocked.coefficient");
-  Task::urgencyInheritCoefficient     = config.getReal ("urgency.inherit.coefficient");
   Task::urgencyAnnotationsCoefficient = config.getReal ("urgency.annotations.coefficient");
   Task::urgencyTagsCoefficient        = config.getReal ("urgency.tags.coefficient");
   Task::urgencyNextCoefficient        = config.getReal ("urgency.next.coefficient");

src/Task.cpp

@@ -30,6 +30,7 @@
 #include <assert.h>
 #ifdef PRODUCT_TASKWARRIOR
 #include <math.h>
+#include <cfloat>
 #endif
 #include <algorithm>
 #ifdef PRODUCT_TASKWARRIOR
@@ -76,7 +77,6 @@ float Task::urgencyActiveCoefficient      = 0.0;
 float Task::urgencyScheduledCoefficient   = 0.0;
 float Task::urgencyWaitingCoefficient     = 0.0;
 float Task::urgencyBlockedCoefficient     = 0.0;
-float Task::urgencyInheritCoefficient     = 0.0;
 float Task::urgencyAnnotationsCoefficient = 0.0;
 float Task::urgencyTagsCoefficient        = 0.0;
 float Task::urgencyNextCoefficient        = 0.0;
@@ -1621,7 +1621,6 @@ float Task::urgency_c () const
   value += fabsf (Task::urgencyDueCoefficient)         > epsilon ? (urgency_due ()         * Task::urgencyDueCoefficient)         : 0.0;
   value += fabsf (Task::urgencyBlockingCoefficient)    > epsilon ? (urgency_blocking ()    * Task::urgencyBlockingCoefficient)    : 0.0;
   value += fabsf (Task::urgencyAgeCoefficient)         > epsilon ? (urgency_age ()         * Task::urgencyAgeCoefficient)         : 0.0;
-  value += fabsf (Task::urgencyInheritCoefficient)     > epsilon ? (urgency_inherit ()     * Task::urgencyInheritCoefficient)     : 0.0;
 
   // Tag- and project-specific coefficients.
   for (auto& var : Task::coefficients)
@@ -1686,6 +1685,17 @@ float Task::urgency_c () const
       }
     }
   }
+
+  if (is_blocking && context.config.getBoolean ("urgency.inherit"))
+  {
+    float prev = value;
+    value = std::max (value, urgency_inherit ());
+
+    // This is a hackish way of making sure parent tasks are sorted above
+    // child tasks.  For reports that hide blocked tasks, this is not needed.
+    if (prev < value)
+      value += 0.01;
+  }
 #endif
 
   return value;
@@ -1708,28 +1718,16 @@ float Task::urgency ()
 ////////////////////////////////////////////////////////////////////////////////
 float Task::urgency_inherit () const
 {
-  if (!is_blocking)
-    return 0.0;
-
   // Calling dependencyGetBlocked is rather expensive.
   // It is called recursively for each dependency in the chain here.
   std::vector <Task> blocked;
   dependencyGetBlocked (*this, blocked);
 
-  float v = 0.0;
+  float v = FLT_MIN;
   for (auto& task : blocked)
   {
-    // urgency_blocked, _blocking, _project and _tags left out.
-    v += task.urgency_active ();
-    v += task.urgency_age ();
-    v += task.urgency_annotations ();
-    v += task.urgency_due ();
-    v += task.urgency_next ();
-    v += task.urgency_scheduled ();
-    v += task.urgency_waiting ();
-
-    // Inherit from all parent tasks in the dependency chain recursively.
-    v += task.urgency_inherit ();
+    // Find highest urgency in all blocked tasks.
+    v = std::max (v, task.urgency ());
   }
 
   return v;

src/Task.h

@@ -48,7 +48,6 @@ public:
   static float urgencyScheduledCoefficient;
   static float urgencyWaitingCoefficient;
   static float urgencyBlockedCoefficient;
-  static float urgencyInheritCoefficient;
   static float urgencyAnnotationsCoefficient;
   static float urgencyTagsCoefficient;
   static float urgencyNextCoefficient;

src/commands/CmdShow.cpp

@@ -194,7 +194,6 @@ int CmdShow::execute (std::string& output)
     " urgency.annotations.coefficient"
     " urgency.blocked.coefficient"
     " urgency.blocking.coefficient"
-    " urgency.inherit.coefficient"
     " urgency.due.coefficient"
     " urgency.next.coefficient"
     " urgency.project.coefficient"
@@ -202,6 +201,7 @@ int CmdShow::execute (std::string& output)
     " urgency.waiting.coefficient"
     " urgency.age.coefficient"
     " urgency.age.max"
+    " urgency.inherit"
     " verbose"
     " weekstart"
     " xterm.title"

src/legacy.cpp

@@ -133,6 +133,10 @@ std::string legacyCheckForDeprecatedVariables ()
     // Deprecated іn 2.4.0.
     if (it.first == "alias._query")
       deprecated.push_back (it.first);
+
+    // Deprecated in 2.4.5.
+    if (it.first == "urgency.inherit.coefficient")
+        deprecated.push_back (it.first);
   }
 
   std::stringstream out;

test/urgency_inherit.t

@@ -0,0 +1,84 @@
+#!/usr/bin/env python2.7
+# -*- coding: utf-8 -*-
+###############################################################################
+#
+# Copyright 2006 - 2015, Paul Beckingham, Federico Hernandez.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+# http://www.opensource.org/licenses/mit-license.php
+#
+###############################################################################
+
+import sys
+import os
+import unittest
+import json
+# Ensure python finds the local simpletap module
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
+
+from basetest import Task, TestCase
+
+
+class TestUrgencyInherit(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.t = Task()
+
+        cls.t.config("urgency.age.coefficient", "0.0")
+        cls.t.config("urgency.blocked.coefficient", "0.0")
+        cls.t.config("urgency.blocking.coefficient", "0.0")
+
+        cls.t("add one")
+        cls.t("add two dep:1")
+        cls.t("add three dep:2 +next due:today-1year")
+
+    def get_tasks(self):
+        tasks = json.loads(self.t("rc.json.array=1 export")[1])
+
+        r = {}
+        for task in tasks:
+            # Make available by ID. Discards non-pending tasks.
+            if task["id"] != 0:
+                r[task["id"]] = task
+
+        return r
+
+    def test_urgency_inherit_off(self):
+        """No urgency inheritance when switched off"""
+        self.t.config("urgency.inherit", "off")
+        tl = self.get_tasks()
+        self.assertTrue(tl[1]["urgency"] <= tl[2]["urgency"] < tl[3]["urgency"])
+
+    def test_gc_off_mod(self):
+        """Biggest urgency is inherited recursively"""
+        self.t.config("urgency.inherit", "off")
+        tl = self.get_tasks()
+        oldmax = max(tl[1]["urgency"], tl[2]["urgency"], tl[3]["urgency"])
+        self.t.config("urgency.inherit", "on")
+        tl = self.get_tasks()
+        self.assertTrue(oldmax <= tl[3]["urgency"])
+        self.assertTrue(tl[1]["urgency"] >= tl[2]["urgency"] >= tl[3]["urgency"])
+
+
+if __name__ == "__main__":
+    from simpletap import TAPTestRunner
+    unittest.main(testRunner=TAPTestRunner())
+
+# vim: ai sts=4 et sw=4