Rewrite the way interval reports are generated

5 files changed, 110 insertions, 87 deletions

diff --git a/src/chain.cc b/src/chain.cc
index 450e3758..61388840 100644
--- a/src/chain.cc
+++ b/src/chain.cc
@@ -217,13 +217,11 @@ post_handler_ptr chain_post_handlers(post_handler_ptr base_handler,
 
   // interval_posts groups posts together based on a time period, such as
   // weekly or monthly.
-  if (report.HANDLED(period_)) {
+  if (report.HANDLED(period_))
     handler.reset(new interval_posts(handler, expr,
                                      report.HANDLER(period_).str(),
                                      report.HANDLED(exact),
                                      report.HANDLED(empty)));
-    handler.reset(new sort_posts(handler, "date"));
-  }
 
   if (report.HANDLED(date_))
     handler.reset(new transfer_details(handler, transfer_details::SET_DATE,
diff --git a/src/filters.cc b/src/filters.cc
index fbef1cd8..fa1f6fa2 100644
--- a/src/filters.cc
+++ b/src/filters.cc
@@ -904,69 +904,104 @@ void subtotal_posts::operator()(post_t& post)
 
 void interval_posts::report_subtotal(const date_interval_t& ival)
 {
-  if (last_post && ival) {
-    if (exact_periods)
-      subtotal_posts::report_subtotal();
-    else
-      subtotal_posts::report_subtotal(NULL, ival);
-  }
+  if (exact_periods)
+    subtotal_posts::report_subtotal();
+  else
+    subtotal_posts::report_subtotal(NULL, ival);
+}
 
-  last_post = NULL;
+namespace {
+  struct sort_posts_by_date {
+    bool operator()(post_t * left, post_t * right) const {
+      return left->date() < right->date();
+    }
+  };
 }
 
 void interval_posts::operator()(post_t& post)
 {
-  DEBUG("filters.interval", "Considering post with amount " << post.amount);
-#if defined(DEBUG_ON)
-  DEBUG("filters.interval", "interval is:");
-  debug_interval(interval);
-#endif
-  if (! interval.find_period(post.date())) {
-    DEBUG("filters.interval", "Post does not fall within period");
+  // If there is a duration (such as weekly), we must generate the
+  // report in two passes.  Otherwise, we only have to check whether the
+  // post falls within the reporting period.
+
+  if (interval.duration) {
+    all_posts.push_back(&post);
+  }
+  else if (interval.find_period(post.date()))
+    item_handler<post_t>::operator()(post);
+}
+
+void interval_posts::flush()
+{
+  if (! interval.duration) {
+    item_handler<post_t>::flush();
     return;
   }
 
-  if (interval.duration) {
-    DEBUG("filters.interval", "There is an interval duration");
-    if (interval != last_interval) {
-#if defined(DEBUG_ON)
-      DEBUG("filters.interval", "interval != last_interval, so reporting");
-      DEBUG("filters.interval", "last_interval is:");
-      debug_interval(last_interval);
-#endif
-      report_subtotal(last_interval);
+  // Sort all the postings we saw by date ascending
+  std::stable_sort(all_posts.begin(), all_posts.end(),
+                   sort_posts_by_date());
 
-      if (generate_empty_posts) {
-        for (++last_interval; last_interval < interval; ++last_interval) {
-          // Generate a null posting, so the intervening periods can be
-          // seen when -E is used, or if the calculated amount ends up being
-          // non-zero
-          xact_t& null_xact = temps.create_xact();
-          null_xact._date = last_interval.inclusive_end();
+  // Determine the beginning interval by using the earliest post
+  if (! interval.find_period(all_posts.front()->date()))
+    throw_(std::logic_error, _("Failed to find period for interval report"));
 
-          post_t& null_post = temps.create_post(null_xact, empty_account);
-          null_post.add_flags(POST_CALCULATED);
-          null_post.amount = 0L;
+  // Walk the interval forward reporting all posts within each one
+  // before moving on, until we reach the end of all_posts
+  bool saw_posts = false;
+  for (std::deque<post_t *>::iterator i = all_posts.begin();
+       i != all_posts.end(); ) {
+    post_t * post(*i);
 
-          last_post = &null_post;
-          subtotal_posts::operator()(null_post);
+    DEBUG("filters.interval",
+          "Considering post " << post->date() << " = " << post->amount);
+#if defined(DEBUG_ON)
+    DEBUG("filters.interval", "interval is:");
+    debug_interval(interval);
+#endif
+    assert(! interval.finish || post->date() < *interval.finish);
 
-          report_subtotal(last_interval);
-        }
-        assert(last_interval <= interval);
-      } else {
-        DEBUG("filters.interval", "Setting last_interval = interval");
-        last_interval = interval;
+    if (interval.within_period(post->date())) {
+      DEBUG("filters.interval", "Calling subtotal_posts::operator()");
+      subtotal_posts::operator()(*post);
+      ++i;
+      saw_posts = true;
+    } else {
+      if (saw_posts) {
+        DEBUG("filters.interval",
+              "Calling subtotal_posts::report_subtotal()");
+        report_subtotal(interval);
+        saw_posts = false;
       }
+      else if (generate_empty_posts) {
+        // Generate a null posting, so the intervening periods can be
+        // seen when -E is used, or if the calculated amount ends up
+        // being non-zero
+        xact_t& null_xact = temps.create_xact();
+        null_xact._date = interval.inclusive_end();
+
+        post_t& null_post = temps.create_post(null_xact, empty_account);
+        null_post.add_flags(POST_CALCULATED);
+        null_post.amount = 0L;
+
+        subtotal_posts::operator()(null_post);
+        report_subtotal(interval);
+      }
+
+      DEBUG("filters.interval", "Advancing interval");
+      ++interval;
     }
-    DEBUG("filters.interval", "Calling subtotal_posts::operator()");
-    subtotal_posts::operator()(post);
-  } else {
-    DEBUG("filters.interval", "There is no interval duration");
-    item_handler<post_t>::operator()(post);
   }
 
-  last_post = &post;
+  // If the last postings weren't reported, do so now.
+  if (saw_posts) {
+    DEBUG("filters.interval",
+          "Calling subtotal_posts::report_subtotal() at end");
+    report_subtotal(interval);
+  }
+    
+  // Tell our parent class to flush
+  subtotal_posts::flush();
 }
 
 void posts_as_equity::report_subtotal()
diff --git a/src/filters.h b/src/filters.h
index 2e51c91c..c972de82 100644
--- a/src/filters.h
+++ b/src/filters.h
@@ -655,11 +655,11 @@ protected:
   typedef std::pair<string, acct_value_t> values_pair;
 
 protected:
-  expr_t&             amount_expr;
-  values_map          values;
-  optional<string>    date_format;
-  temporaries_t       temps;
-  std::list<post_t *> component_posts;
+  expr_t&              amount_expr;
+  values_map           values;
+  optional<string>     date_format;
+  temporaries_t        temps;
+  std::deque<post_t *> component_posts;
 
 public:
   subtotal_posts(post_handler_ptr handler, expr_t& _amount_expr,
@@ -697,12 +697,12 @@ class interval_posts : public subtotal_posts
 {
   date_interval_t start_interval;
   date_interval_t interval;
-  date_interval_t last_interval;
-  post_t *        last_post;
   account_t *     empty_account;
   bool            exact_periods;
   bool            generate_empty_posts;
 
+  std::deque<post_t *> all_posts;
+
   interval_posts();
 
 public:
@@ -713,8 +713,7 @@ public:
                  bool                   _exact_periods        = false,
                  bool                   _generate_empty_posts = false)
     : subtotal_posts(_handler, amount_expr), start_interval(_interval),
-      interval(start_interval), last_post(NULL),
-      exact_periods(_exact_periods),
+      interval(start_interval), exact_periods(_exact_periods),
       generate_empty_posts(_generate_empty_posts) {
     TRACE_CTOR(interval_posts,
                "post_handler_ptr, expr_t&, date_interval_t, bool, bool");
@@ -744,28 +743,11 @@ public:
   }
 #endif
 
-  virtual void flush() {
-    if (last_post && interval.duration) {
-      DEBUG("filters.interval", "There is a last_post and an interval.duration");
-      if (interval != last_interval) {
-#if defined(DEBUG_ON)
-        DEBUG("filters.interval", "interval != last_interval, so reporting");
-        DEBUG("filters.interval", "interval is:");
-        debug_interval(interval);
-        DEBUG("filters.interval", "last_interval is:");
-        debug_interval(last_interval);
-#endif
-        report_subtotal(last_interval);
-      }
-      subtotal_posts::flush();
-    }
-  }
   virtual void operator()(post_t& post);
+  virtual void flush();
 
   virtual void clear() {
-    interval      = start_interval;
-    last_interval = date_interval_t();
-    last_post     = NULL;
+    interval  = start_interval;
 
     subtotal_posts::clear();
     create_accounts();
diff --git a/src/times.cc b/src/times.cc
index dd10a508..9712c2ee 100644
--- a/src/times.cc
+++ b/src/times.cc
@@ -1305,7 +1305,7 @@ void date_interval_t::stabilize(const optional<date_t>& date)
         date_interval_t next_interval(*this);
         ++next_interval;
 
-        if (next_interval.start && *next_interval.start < *date) {
+        if (next_interval.start && *next_interval.start <= *date) {
           *this = next_interval;
         } else {
           end_of_duration = none;
@@ -1355,7 +1355,8 @@ void date_interval_t::stabilize(const optional<date_t>& date)
   }
 }
 
-bool date_interval_t::find_period(const date_t& date)
+bool date_interval_t::find_period(const date_t& date,
+                                  const bool    allow_shift)
 {
   stabilize(date);
 
@@ -1405,9 +1406,6 @@ bool date_interval_t::find_period(const date_t& date)
 #endif
 
   while (date >= scan && (! finish || scan < *finish)) {
-    DEBUG("times.interval", "date        = " << date);
-    DEBUG("times.interval", "end_of_scan = " << end_of_scan);
-
     if (date < end_of_scan) {
       start           = scan;
       end_of_duration = end_of_scan;
@@ -1420,9 +1418,15 @@ bool date_interval_t::find_period(const date_t& date)
 
       return true;
     }
+    else if (! allow_shift) {
+      break;
+    }
 
     scan        = duration->add(scan);
     end_of_scan = duration->add(scan);
+
+    DEBUG("times.interval", "scan        = " << scan);
+    DEBUG("times.interval", "end_of_scan = " << end_of_scan);
   }
 
   DEBUG("times.interval", "false: failed scan");
diff --git a/src/times.h b/src/times.h
index a2680ae3..bc462efa 100644
--- a/src/times.h
+++ b/src/times.h
@@ -578,10 +578,14 @@ public:
     return start;
   }
 
-  /** Find the current or next period containing date.  Returns true if the
-      date_interval_t object has been altered to reflect the interval
-      containing date, or false if no such period can be found. */
-  bool find_period(const date_t& date = CURRENT_DATE());
+  /** Find the current or next period containing date.  Returns false if
+      no such period can be found.  If allow_shift is true, the default,
+      then the interval may be shifted in time to find the period. */
+  bool find_period(const date_t& date        = CURRENT_DATE(),
+                   const bool    allow_shift = true);
+  bool within_period(const date_t& date = CURRENT_DATE()) {
+    return find_period(date, false);
+  }
 
   optional<date_t> inclusive_end() const {
     if (end_of_duration)