Feature - #156

+ Task now supports both a 'side' and 'diff' style of undo.
+ Undo now observes the 'color.undo.before' and 'color.undo.after'
  configuration variables.

ChangeLog

@@ -32,6 +32,8 @@
    such as '3 mths' or '24 hrs'.
  + The ghistory graph bars can now be colored with 'color.history.add',
    'color.history.done' and 'color.history.delete' configuration variables.
+ + Added feature #156, so that task supports both a 'side' and 'diff' style
+   of undo.
  + Fixed bug #406 so that task now includes command aliases in the _commands
    helper command used by shell completion scripts.
  + Fixed bug #211 - it was unclear which commands modify a task description.

doc/man/taskrc.5

@@ -280,6 +280,12 @@ weekly recurring task is added with a due date of tomorrow, and recurrence.limit
 is set to 2, then a report will list 2 pending recurring tasks, one for tomorrow,
 and one for a week from tomorrow.
 
+.TP
+.B undo.style=side
+When the 'undo' command is run, task presents a before and after comparison of the
+data.  This can be in either the 'side' style, which compares values side-by-side
+in a table, or 'diff' style, which uses a format similar to the 'diff' command.
+
 .TP
 .B debug=off
 Task has a debug mode that causes diagnostic output to be displayed.  Typically
@@ -628,6 +634,16 @@ Colors the bars on the ghistory report graphs.  Defaults to red, green and
 yellow bars.
 .RE
 
+.TP
+.B color.undo.before=red
+.RE
+.br
+.B color.undo.after=green
+.RS
+Colors used by the undo command, to indicate the values both before and after
+a change that is to be reverted.
+.RE
+
 .SS SHADOW FILE
 
 .TP

src/Att.cpp

@@ -50,6 +50,7 @@ static const char* internalNames[] =
   "limit",
   "status",
   "description",
+  // Note that annotations are not listed.
 };
 
 static const char* modifiableNames[] =
@@ -758,6 +759,19 @@ int Att::value_int () const
   return atoi (mValue.c_str ());
 }
 
+////////////////////////////////////////////////////////////////////////////////
+void Att::allNames (std::vector <std::string>& all)
+{
+  all.clear ();
+
+  unsigned int i;
+  for (i = 0; i < NUM_INTERNAL_NAMES; ++i)
+    all.push_back (internalNames[i]);
+
+  for (i = 0; i < NUM_MODIFIABLE_NAMES; ++i)
+    all.push_back (modifiableNames[i]);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 void Att::value_int (int value)
 {

src/Att.h

@@ -70,6 +70,8 @@ public:
   int value_int () const;
   void value_int (int);
 
+  static void allNames (std::vector <std::string>&);
+
 private:
   void enquote (std::string&) const;
   void dequote (std::string&) const;

src/Config.cpp

@@ -76,6 +76,7 @@ std::string Config::defaults =
   "tag.indicator=+                        # What to show as a tag indicator\n"
   "recurrence.indicator=R                 # What to show as a task recurrence indicator\n"
   "recurrence.limit=1                     # Number of future recurring pending tasks\n"
+  "undo.style=side                        # Undo style - can be 'side', or 'diff'\n"
   "\n"
   "# Dates\n"
   "dateformat=m/d/Y                       # Preferred input and display date format\n"
@@ -109,6 +110,8 @@ std::string Config::defaults =
   "color.history.add=on red                      # Color of added tasks in ghistory report\n"
   "color.history.done=on green                   # Color of completed tasks in ghistory report\n"
   "color.history.delete=on yellow                # Color of deleted tasks in ghistory report\n"
+  "color.undo.before=red                         # Color of values before a change\n"
+  "color.undo.after=green                        # Color of values after a change\n"
   "#color.debug=magenta                          # Color of diagnostic output\n"
   "\n"
   "# The following rules are presented in their order of precedence.\n"

src/TDB.cpp

@@ -664,97 +664,236 @@ void TDB::undo ()
   }
 
   Date lastChange (atoi (when.c_str ()));
-  std::cout << std::endl
-            << "The last modification was made "
-            << lastChange.toString ()
-            << std::endl;
 
-  // Attributes are all there is, so figure the different attribute names
-  // between before and after.
-  Table table;
-  table.setTableWidth (context.getWidth ());
-  table.setTableIntraPadding (2);
-  table.addColumn (" ");
-  table.addColumn ("Prior Values");
-  table.addColumn ("Current Values");
-  table.setColumnUnderline (1);
-  table.setColumnUnderline (2);
-  table.setColumnWidth (0, Table::minimum);
-  table.setColumnWidth (1, Table::flexible);
-  table.setColumnWidth (2, Table::flexible);
+  // Set the colors.
+  Color color_red   (context.config.get ("color.undo.before"));
+  Color color_green (context.config.get ("color.undo.after"));
 
-  Task after (current);
-
-  if (prior != "")
+  if (context.config.get ("undo.style") == "side")
   {
-    Task before (prior);
+    std::cout << std::endl
+              << "The last modification was made "
+              << lastChange.toString ()
+              << std::endl;
 
-    std::vector <std::string> beforeAtts;
-    foreach (att, before)
-      beforeAtts.push_back (att->first);
+    // Attributes are all there is, so figure the different attribute names
+    // between before and after.
+    Table table;
+    table.setTableWidth (context.getWidth ());
+    table.setTableIntraPadding (2);
+    table.addColumn (" ");
+    table.addColumn ("Prior Values");
+    table.addColumn ("Current Values");
+    table.setColumnUnderline (1);
+    table.setColumnUnderline (2);
+    table.setColumnWidth (0, Table::minimum);
+    table.setColumnWidth (1, Table::flexible);
+    table.setColumnWidth (2, Table::flexible);
 
-    std::vector <std::string> afterAtts;
-    foreach (att, after)
-      afterAtts.push_back (att->first);
+    Task after (current);
 
-    std::vector <std::string> beforeOnly;
-    std::vector <std::string> afterOnly;
-    listDiff (beforeAtts, afterAtts, beforeOnly, afterOnly);
-
-    int row;
-    foreach (name, beforeOnly)
+    if (prior != "")
     {
-      row = table.addRow ();
-      table.addCell (row, 0, *name);
-      table.addCell (row, 1, renderAttribute (*name, before.get (*name)));
-      table.setCellColor (row, 1, Color (Color::red));
+      Task before (prior);
+
+      std::vector <std::string> beforeAtts;
+      foreach (att, before)
+        beforeAtts.push_back (att->first);
+
+      std::vector <std::string> afterAtts;
+      foreach (att, after)
+        afterAtts.push_back (att->first);
+
+      std::vector <std::string> beforeOnly;
+      std::vector <std::string> afterOnly;
+      listDiff (beforeAtts, afterAtts, beforeOnly, afterOnly);
+
+      int row;
+      foreach (name, beforeOnly)
+      {
+        row = table.addRow ();
+        table.addCell (row, 0, *name);
+        table.addCell (row, 1, renderAttribute (*name, before.get (*name)));
+        table.setCellColor (row, 1, color_red);
+      }
+
+      foreach (name, before)
+      {
+        std::string priorValue   = before.get (name->first);
+        std::string currentValue = after.get  (name->first);
+
+        if (currentValue != "")
+        {
+          row = table.addRow ();
+          table.addCell (row, 0, name->first);
+          table.addCell (row, 1, renderAttribute (name->first, priorValue));
+          table.addCell (row, 2, renderAttribute (name->first, currentValue));
+
+          if (priorValue != currentValue)
+          {
+            table.setCellColor (row, 1, color_red);
+            table.setCellColor (row, 2, color_green);
+          }
+        }
+      }
+
+      foreach (name, afterOnly)
+      {
+        row = table.addRow ();
+        table.addCell (row, 0, *name);
+        table.addCell (row, 2, renderAttribute (*name, after.get (*name)));
+        table.setCellColor (row, 2, color_green);
+      }
     }
-
-    foreach (name, before)
+    else
     {
-      std::string priorValue   = before.get (name->first);
-      std::string currentValue = after.get  (name->first);
-
-      if (currentValue != "")
+      int row;
+      foreach (name, after)
       {
         row = table.addRow ();
         table.addCell (row, 0, name->first);
-        table.addCell (row, 1, renderAttribute (name->first, priorValue));
-        table.addCell (row, 2, renderAttribute (name->first, currentValue));
+        table.addCell (row, 2, renderAttribute (name->first, after.get (name->first)));
+        table.setCellColor (row, 2, color_green);
+      }
+    }
 
-        if (priorValue != currentValue)
+    std::cout << std::endl
+              << table.render ()
+              << std::endl;
+  }
+
+  // This style looks like this:
+  //  --- before    2009-07-04 00:00:25.000000000 +0200
+  //  +++ after    2009-07-04 00:00:45.000000000 +0200
+  //
+  // - name: old           // att deleted
+  // + name:
+  //
+  // - name: old           // att changed
+  // + name: new
+  //
+  // - name:
+  // + name: new           // att added
+  //
+  else if (context.config.get ("undo.style") == "diff")
+  {
+    // Create reference tasks.
+    Task before;
+    if (prior != "")
+      before.parse (prior);
+
+    Task after (current);
+
+    // Generate table header.
+    Table table;
+    table.setTableWidth (context.getWidth ());
+    table.setTableIntraPadding (2);
+    table.addColumn (" ");
+    table.addColumn (" ");
+    table.setColumnWidth (0, Table::minimum);
+    table.setColumnWidth (1, Table::flexible);
+    table.setColumnJustification (0, Table::right);
+    table.setColumnJustification (1, Table::left);
+
+    int row = table.addRow ();
+    table.addCell (row, 0, "--- before");
+    table.addCell (row, 1, "Previous state that undo will restore");
+    table.setRowColor (row, color_red);
+
+    row = table.addRow ();
+    table.addCell (row, 0, "+++ after ");  // Note trailing space.
+    table.addCell (row, 1, "Change made: " + lastChange.toStringWithTime (context.config.get ("dateformat")));
+    table.setRowColor (row, color_green);
+
+    table.addRow ();
+
+    // Add rows to table showing diffs.
+    std::vector <std::string> all;
+    Att::allNames (all);
+
+    // Now factor in the annotation attributes.
+    Task::iterator it;
+    for (it = before.begin (); it != before.end (); ++it)
+      if (it->first.substr (0, 11) == "annotation_")
+        all.push_back (it->first);
+
+    for (it = after.begin (); it != after.end (); ++it)
+      if (it->first.substr (0, 11) == "annotation_")
+        all.push_back (it->first);
+
+    // Now render all the attributes.
+    std::sort (all.begin (), all.end ());
+
+    std::string before_att;
+    std::string after_att;
+    std::string last_att;
+    foreach (a, all)
+    {
+      if (*a != last_att)  // Skip duplicates.
+      {
+        last_att = *a;
+
+        before_att = before.get (*a);
+        after_att  = after.get (*a);
+
+        // Don't report different uuid.
+        // Show nothing if values are the unchanged.
+        if (*a == "uuid" ||
+            before_att == after_att)
         {
-          table.setCellColor (row, 1, Color (Color::red));
-          table.setCellColor (row, 2, Color (Color::green));
+          row = table.addRow ();
+          table.addCell (row, 0, *a + ":");
+          table.addCell (row, 1, before_att);
+        }
+
+        // Attribute deleted.
+        else if (before_att != "" && after_att == "")
+        {
+          row = table.addRow ();
+          table.addCell (row, 0, "-" + *a + ":");
+          table.addCell (row, 1, before_att);
+          table.setRowColor (row, color_red);
+
+          row = table.addRow ();
+          table.addCell (row, 0, "+" + *a + ":");
+          table.setRowColor (row, color_green);
+        }
+
+        // Attribute added.
+        else if (before_att == "" && after_att != "")
+        {
+          row = table.addRow ();
+          table.addCell (row, 0, "-" + *a + ":");
+          table.setRowColor (row, color_red);
+
+          row = table.addRow ();
+          table.addCell (row, 0, "+" + *a + ":");
+          table.addCell (row, 1, after_att);
+          table.setRowColor (row, color_green);
+        }
+
+        // Attribute changed.
+        else
+        {
+          row = table.addRow ();
+          table.addCell (row, 0, "-" + *a + ":");
+          table.addCell (row, 1, before_att);
+          table.setRowColor (row, color_red);
+
+          row = table.addRow ();
+          table.addCell (row, 0, "+" + *a + ":");
+          table.addCell (row, 1, after_att);
+          table.setRowColor (row, color_green);
         }
       }
     }
 
-    foreach (name, afterOnly)
-    {
-      row = table.addRow ();
-      table.addCell (row, 0, *name);
-      table.addCell (row, 2, renderAttribute (*name, after.get (*name)));
-      table.setCellColor (row, 2, Color (Color::green));
-    }
-  }
-  else
-  {
-    int row;
-    foreach (name, after)
-    {
-      row = table.addRow ();
-      table.addCell (row, 0, name->first);
-      table.addCell (row, 2, renderAttribute (name->first, after.get (name->first)));
-      table.setCellColor (row, 2, Color (Color::green));
-    }
+    std::cout << std::endl
+              << table.render ()
+              << std::endl;
   }
 
-  // Confirm.
-  std::cout << std::endl
-            << table.render ()
-            << std::endl;
-
+  // Output displayed, now confirm.
   if (!confirm ("The undo command is not reversible.  Are you sure you want to undo the last update?"))
     throw std::string ("No changes made.");
 

src/command.cpp

@@ -659,8 +659,8 @@ int handleShow (std::string &outs)
       "color.alternate color.calendar.today color.calendar.due color.calendar.due.today "
       "color.calendar.overdue color.calendar.weekend color.calendar.holiday "
       "color.calendar.weeknumber color.summary.background color.summary.bar "
-      "color.history.add color.history.done color.history.delete "
-      "confirmation curses data.location dateformat dateformat.holiday "
+      "color.history.add color.history.done color.history.delete color.undo.before "
+      "color.undo.after confirmation curses data.location dateformat dateformat.holiday "
       "dateformat.report dateformat.annotation debug default.command "
       "default.priority default.project defaultwidth due locale displayweeknumber "
       "export.ical.class echo.command fontunderline locking monthsperline nag "
@@ -668,6 +668,7 @@ int handleShow (std::string &outs)
       "import.synonym.id import.synonym.uuid complete.all.projects "
       "complete.all.tags search.case.sensitive hooks active.indicator tag.indicator "
       "recurrence.indicator recurrence.limit list.all.projects list.all.tags "
+      "undo.style "
 #ifdef FEATURE_SHELL
       "shell.prompt "
 #endif

src/tests/att.t.cpp

@@ -33,7 +33,7 @@ Context context;
 ////////////////////////////////////////////////////////////////////////////////
 int main (int argc, char** argv)
 {
-  UnitTest t (97);
+  UnitTest t (99);
 
   Att a;
   t.notok (a.valid ("name"),            "Att::valid name         -> fail");
@@ -289,6 +289,16 @@ int main (int argc, char** argv)
   t.ok (Att::validModifiableName ("until"),     "modifiable until");
   t.ok (Att::validModifiableName ("wait"),      "modifiable wait");
 
+  // Att::allNames
+  std::vector <std::string> all;
+  Att::allNames (all);
+
+  std::vector <std::string>::iterator it;
+  it = std::find (all.begin (), all.end (), "uuid");
+  t.ok (it != all.end (), "internal name 'uuid' found in Att::allNames");
+  it = std::find (all.begin (), all.end (), "project");
+  t.ok (it != all.end (), "modifiable name 'project' found in Att::allNames");
+
   return 0;
 }