From 9e6c6ecb93b65f5b13aba76f760e825fa9256f93 Mon Sep 17 00:00:00 2001
From: Wilhelm Schuermann <wimschuermann@googlemail.com>
Date: Sat, 28 Mar 2015 10:12:19 +0100
Subject: [PATCH] TW-1583: Invalid ID displayed after done/delete

- Fix completed/deleted tasks getting an ID when GC is going to be run,
  previously resulting in invalid IDs being displayed in reports that
  show non-pending tasks.

A side effect of this fix is that it is sometimes not possible to
filter by ID when running a report right after calling done/delete.
This problem existed before; this change makes it happen on the first
report instead of the second, so it is more consistently broken.
Commands that modify tasks are not affected, making this an annoying
yet harmless defect.
---
 ChangeLog       |  2 ++
 src/Context.cpp |  8 ++++++++
 src/Context.h   |  2 ++
 src/TDB2.cpp    | 10 ++++++++--
 test/export.t   |  6 ++++--
 5 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2ec58ba4e..ce5e3c402 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -6,6 +6,8 @@
 - TW-1581 Tasks with dependencies show wrong urgency values for the first
   report run after a task in the dependency chain is completed/deleted (thanks
   to Ulf Eliasson).
+- TW-1583 Invalid ID displayed for first report after done/delete (thanks to
+  Ulf Eliasson).
 - Setting 'bulk' to zero is interpreted as infinity, which means there is no
   amount of changes that is considered dangerous (thanks to Tomas Babej).
 
diff --git a/src/Context.cpp b/src/Context.cpp
index e7adc9b89..78e769dac 100644
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -101,6 +101,7 @@ Context::Context ()
 , dom ()
 , determine_color_use (true)
 , use_color (true)
+, run_gc (true)
 , verbosity_legacy (false)
 , terminal_width (0)
 , terminal_height (0)
@@ -478,7 +479,14 @@ int Context::dispatch (std::string &out)
     // GC is invoked prior to running any command that displays task IDs, if
     // possible.
     if (c->displays_id () && !tdb2.read_only ())
+    {
+      run_gc = true;
       tdb2.gc ();
+    }
+    else
+    {
+      run_gc = false;
+    }
 
 /*
     // Only read-only commands can be run when TDB2 is read-only.
diff --git a/src/Context.h b/src/Context.h
index 5bca1bb08..40d1ed3db 100644
--- a/src/Context.h
+++ b/src/Context.h
@@ -94,6 +94,8 @@ public:
   bool                                determine_color_use;
   bool                                use_color;
 
+  bool                                run_gc;
+
   bool                                verbosity_legacy;
   std::vector <std::string>           verbosity;
   std::vector <std::string>           headers;
diff --git a/src/TDB2.cpp b/src/TDB2.cpp
index 0b35fdfd2..abcafd3a8 100644
--- a/src/TDB2.cpp
+++ b/src/TDB2.cpp
@@ -316,9 +316,15 @@ void TF2::load_tasks ()
       ++line_number;
       Task task (*i);
 
-      // Some tasks gets an ID.
+      // Some tasks get an ID.
       if (_has_ids)
-        task.id = context.tdb2.next_id ();
+      {
+        Task::status status = task.getStatus ();
+        // Completed / deleted tasks in pending.data get an ID if GC is off.
+        if (!context.run_gc ||
+            (status != Task::completed && status != Task::deleted))
+          task.id = context.tdb2.next_id ();
+      }
 
       _tasks.push_back (task);
 
diff --git a/test/export.t b/test/export.t
index 1a666f15f..108347210 100755
--- a/test/export.t
+++ b/test/export.t
@@ -108,8 +108,10 @@ class TestExportCommand(TestCase):
     def test_export_end(self):
         self.t(('1', 'start'))
         self.t.faketime("+5s")
-        self.t(('1', 'done'))
-        self.assertTimestamp(self.export(1)['end'])
+        # After a task is "done" or "deleted", it does not have an ID by which
+        # to filter it anymore. Add a tag to work around this.
+        self.t(('1', 'done', '+workaround'))
+        self.assertTimestamp(self.export('+workaround')['end'])
 
     def test_export_due(self):
         self.t(('1', 'modify', 'due:today'))