Bug #1270

- The 'undo' command is now properly removing backlog entries.

ChangeLog

@@ -66,6 +66,7 @@ Bugs
    Wilk).
  + #1263 The 'waiting' report properly lists only pending tasks with a wait date
    (thanks to Fidel Mato).
+ + #1270 The 'undo' command is now properly removing backlog entries.
  + #1300 Encode/decode pairing is now properly balanced.
  + #1305 Commit hash now available in tarball builds (thanks to Ben Boeckel).
  + #1387 ZSH Auto-Completion dates are not current (thanks to Benjamin Weber).

src/TDB2.cpp

@@ -1293,7 +1293,7 @@ void TDB2::revert ()
     // Revert: task add
     // [1] 0 --> p
     //   - erase from pending
-    //   - if in backlog, erase, else add deletion
+    //   - if in backlog, erase, else cannot undo
     //
     // Revert: task modify
     // [2] p --> p'
@@ -1320,7 +1320,7 @@ void TDB2::revert ()
     // Revert: task log
     // [6] 0 --> c
     //   - erase from completed
-    //   - if in backlog, erase, else add deletion
+    //   - if in backlog, erase, else cannot undo
 
     // Modify other data files accordingly.
     std::vector <std::string> p = pending.get_lines ();
@@ -1338,15 +1338,6 @@ void TDB2::revert ()
     File::write (pending._file._data, p);
     File::write (completed._file._data, c);
     File::write (backlog._file._data, b);
-
-/*
-    // Perhaps user hand-edited the data files?
-    // Perhaps the task was in completed.data, which was still in file format 3?
-    std::cout << format (STRING_TDB2_MISSING_TASK, uuid.substr (6, 36))
-              << "\n"
-              << STRING_TDB2_UNDO_IMPOSSIBLE
-              << "\n";
-*/
   }
   else
     std::cout << STRING_CMD_CONFIG_NO_CHANGE << "\n";
@@ -1386,7 +1377,7 @@ void TDB2::revert_undo (
   // Extract identifying uuid.
   std::string::size_type uuidAtt = current.find ("uuid:\"");
   if (uuidAtt != std::string::npos)
-    uuid = current.substr (uuidAtt, 43); // 43 = uuid:"..."
+    uuid = current.substr (uuidAtt + 6, 36); // "uuid:"<uuid>" --> <uuid>
   else
     throw std::string (STRING_TDB2_MISSING_UUID);
 }
@@ -1398,11 +1389,13 @@ void TDB2::revert_pending (
   const std::string& current,
   const std::string& prior)
 {
+  std::string uuid_att = "uuid:\"" + uuid + "\"";
+
   // is 'current' in pending?
   std::vector <std::string>::iterator task;
   for (task = p.begin (); task != p.end (); ++task)
   {
-    if (task->find (uuid) != std::string::npos)
+    if (task->find (uuid_att) != std::string::npos)
     {
       context.debug ("TDB::revert - task found in pending.data");
 
@@ -1431,13 +1424,15 @@ void TDB2::revert_completed (
   const std::string& current,
   const std::string& prior)
 {
+  std::string uuid_att = "uuid:\"" + uuid + "\"";
+
   // is 'current' in completed?
   std::vector <std::string>::iterator task;
   for (task = c.begin (); task != c.end (); ++task)
   {
-    if (task->find (uuid) != std::string::npos)
+    if (task->find (uuid_att) != std::string::npos)
     {
-      context.debug ("TDB::revert - task found in completed.data");
+      context.debug ("TDB::revert_completed - task found in completed.data");
 
       // Either revert if there was a prior state, or remove the task.
       if (prior != "")
@@ -1450,12 +1445,12 @@ void TDB2::revert_completed (
           c.erase (task);
           p.push_back (prior);
           std::cout << STRING_TDB2_REVERTED << "\n";
-          context.debug ("TDB::revert - task belongs in pending.data");
+          context.debug ("TDB::revert_completed - task belongs in pending.data");
         }
         else
         {
           std::cout << STRING_TDB2_REVERTED << "\n";
-          context.debug ("TDB::revert - task belongs in completed.data");
+          context.debug ("TDB::revert_completed - task belongs in completed.data");
         }
       }
       else
@@ -1463,7 +1458,7 @@ void TDB2::revert_completed (
         c.erase (task);
 
         std::cout << STRING_TDB2_REVERTED << "\n";
-        context.debug ("TDB::revert - task removed");
+        context.debug ("TDB::revert_completed - task removed");
       }
 
       std::cout << STRING_TDB2_UNDO_COMPLETE << "\n";
@@ -1479,21 +1474,38 @@ void TDB2::revert_backlog (
   const std::string& current,
   const std::string& prior)
 {
-  std::vector <std::string>::iterator task;
+  std::string uuid_att = "\"uuid\":\"" + uuid + "\"";
-  for (task = b.begin (); task != b.end (); ++task)
+
+  bool found = false;
+  std::vector <std::string>::reverse_iterator task;
+  for (task = b.rbegin (); task != b.rend (); ++task)
   {
-    if (task->find (uuid) != std::string::npos)
+    if (task->find (uuid_att) != std::string::npos)
     {
-/*
       context.debug ("TDB::revert_backlog - task found in backlog.data");
+      found = true;
 
-//        // Write prior to backlog.data
+      // If this is a new task (no prior), then just remove it from the backlog.
-//        Task json_task (prior);
+      if (current != "" && prior == "")
-//        backlog.add_line (json_task.composeJSON () + "\n");
+      {
+        // Yes, this is what is needed, when you want to erase using a reverse
+        // iterator.
+        b.erase ((++task).base ());
+      }
 
-*/
+      // If this is a modification of some kind, add the prior to the backlog.
+      else
+      {
+        Task t (prior);
+        b.push_back (t.composeJSON ());
+      }
+
+      break;
     }
   }
+
+  if (!found)
+    throw std::string (STRING_TDB2_UNDO_SYNCED);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/en-US.h

@@ -833,6 +833,7 @@
 #define STRING_TDB2_UNDO_COMPLETE    "Undo complete."
 #define STRING_TDB2_MISSING_TASK     "Task with UUID {1} not found in data."
 #define STRING_TDB2_UNDO_IMPOSSIBLE  "No undo possible."
+#define STRING_TDB2_UNDO_SYNCED      "Cannot undo change because the task was already synced.  Modify the task instead."
 
 // text
                                      // A comma-separated list of commands is appended.

src/es-ES.h

@@ -848,6 +848,7 @@
 #define STRING_TDB2_UNDO_COMPLETE    "Deshacer completado."
 #define STRING_TDB2_MISSING_TASK     "Tarea con UUID {1} no encontrada en los datos."
 #define STRING_TDB2_UNDO_IMPOSSIBLE  "No es posible deshacer."
+#define STRING_TDB2_UNDO_SYNCED      "Cannot undo change because the task was already synced.  Modify the task instead."
 
 // text
                                      // Se añade al final una lista de comandos separados por comas.

src/fr-FR.h

@@ -833,6 +833,7 @@
 #define STRING_TDB2_UNDO_COMPLETE    "Undo complete."
 #define STRING_TDB2_MISSING_TASK     "Task with UUID {1} not found in data."
 #define STRING_TDB2_UNDO_IMPOSSIBLE  "No undo possible."
+#define STRING_TDB2_UNDO_SYNCED      "Cannot undo change because the task was already synced.  Modify the task instead."
 
 // text
                                      // A comma-separated list of commands is appended.

src/it-IT.h

@@ -834,6 +834,7 @@
 #define STRING_TDB2_UNDO_COMPLETE    "Undo completato."
 #define STRING_TDB2_MISSING_TASK     "Task con UUID {1} non trovato nei dati."
 #define STRING_TDB2_UNDO_IMPOSSIBLE  "Nessun undo possibile."
+#define STRING_TDB2_UNDO_SYNCED      "Cannot undo change because the task was already synced.  Modify the task instead."
 
 // text
                                      // A comma-separated list of commands is appended.

test/delete.t

@@ -53,7 +53,7 @@ $output = qx{../src/task rc:delete.rc 1 delete 2>&1; ../src/task rc:delete.rc in
 like ($output, qr/Status\s+Deleted\n/, 'Deleted');
 ok (-r 'completed.data', 'completed.data created');
 
-$output = qx{echo 'y' | ../src/task rc:delete.rc undo 2>&1; ../src/task rc:delete.rc info 1 2>&1};
+$output = qx{../src/task rc:delete.rc undo 2>&1; ../src/task rc:delete.rc info 1 2>&1};
 like ($output, qr/Status\s+Pending\n/, 'Pending');
 ok (-r 'completed.data', 'completed.data created');