Improved the --collapse filter

It now takes the --display and --only predicates into account, so that
it never appears to be collapsing single transactions.

There are cases where there are multiple transactions, but the display
or only predicate filters them out, so that if collapse didn't consider
this, the user would wonder why a single transaction was being collapsed
-- since they'd never see that collapse saw more than two.

3 files changed, 37 insertions, 21 deletions

diff --git a/src/chain.cc b/src/chain.cc
index 1d24aafb..c5891f3a 100644
--- a/src/chain.cc
+++ b/src/chain.cc
@@ -40,6 +40,8 @@ xact_handler_ptr chain_xact_handlers(report_t&	      report,
 				     bool             only_preliminaries)
 {
   xact_handler_ptr handler(base_handler);
+  item_predicate   display_predicate;
+  item_predicate   only_predicate;
 
   assert(report.HANDLED(amount_));
   expr_t& expr(report.HANDLER(amount_).expr);
@@ -58,11 +60,11 @@ xact_handler_ptr chain_xact_handlers(report_t&	      report,
 
     // filter_xacts will only pass through xacts matching the
     // `display_predicate'.
-    if (report.HANDLED(display_))
-      handler.reset(new filter_xacts
-		    (handler, item_predicate(report.HANDLER(display_).str(),
-					     report.what_to_keep()),
-		     report));
+    if (report.HANDLED(display_)) {
+      display_predicate = item_predicate(report.HANDLER(display_).str(),
+					 report.what_to_keep());
+      handler.reset(new filter_xacts(handler, display_predicate, report));
+    }
 
     // calc_xacts computes the running total.  When this appears will
     // determine, for example, whether filtered xacts are included or excluded
@@ -72,11 +74,11 @@ xact_handler_ptr chain_xact_handlers(report_t&	      report,
 
   // filter_xacts will only pass through xacts matching the
   // `secondary_predicate'.
-  if (report.HANDLED(only_))
-    handler.reset(new filter_xacts
-		  (handler, item_predicate(report.HANDLER(only_).str(),
-					   report.what_to_keep()),
-		   report));
+  if (report.HANDLED(only_)) {
+    only_predicate = item_predicate(report.HANDLER(only_).str(),
+				    report.what_to_keep());
+    handler.reset(new filter_xacts(handler, only_predicate, report));
+  }
 
   // sort_xacts will sort all the xacts it sees, based on the `sort_order'
   // value expression.
@@ -100,6 +102,7 @@ xact_handler_ptr chain_xact_handlers(report_t&	      report,
     // with a subtotaled xact for each commodity used.
     if (report.HANDLED(collapse))
       handler.reset(new collapse_xacts(handler, expr,
+				       display_predicate, only_predicate,
 				       report.HANDLED(collapse_if_zero)));
   }
 
diff --git a/src/filters.cc b/src/filters.cc
index 22231d18..276ecaba 100644
--- a/src/filters.cc
+++ b/src/filters.cc
@@ -291,7 +291,13 @@ void collapse_xacts::report_subtotal()
 {
   assert(count >= 1);
 
-  if (count == 1) {
+  std::size_t displayed_count = 0;
+  foreach (xact_t * xact, component_xacts) {
+    if (only_predicate(*xact) && display_predicate(*xact))
+      displayed_count++;
+  }  
+
+  if (displayed_count == 1) {
     item_handler<xact_t>::operator()(*last_xact);
   }
   else if (only_collapse_if_zero && ! subtotal.is_zero()) {
diff --git a/src/filters.h b/src/filters.h
index 5e72fb4d..66d0dd2e 100644
--- a/src/filters.h
+++ b/src/filters.h
@@ -379,13 +379,15 @@ public:
  */
 class collapse_xacts : public item_handler<xact_t>
 {
-  expr_t&     amount_expr;
-  value_t     subtotal;
-  std::size_t count;
-  entry_t *   last_entry;
-  xact_t *    last_xact;
-  account_t   totals_account;
-  bool        only_collapse_if_zero;
+  expr_t&	 amount_expr;
+  item_predicate display_predicate;
+  item_predicate only_predicate;
+  value_t	 subtotal;
+  std::size_t	 count;
+  entry_t *	 last_entry;
+  xact_t *	 last_xact;
+  account_t	 totals_account;
+  bool		 only_collapse_if_zero;
 
   std::list<entry_t>  entry_temps;
   std::list<xact_t>   xact_temps;
@@ -394,10 +396,15 @@ class collapse_xacts : public item_handler<xact_t>
   collapse_xacts();
 
 public:
-  collapse_xacts(xact_handler_ptr handler, expr_t& _amount_expr,
-		 bool _only_collapse_if_zero = false)
+  collapse_xacts(xact_handler_ptr handler,
+		 expr_t&	  _amount_expr,
+		 item_predicate   _display_predicate,
+		 item_predicate   _only_predicate,
+		 bool             _only_collapse_if_zero = false)
     : item_handler<xact_t>(handler), amount_expr(_amount_expr),
-      count(0), last_entry(NULL), last_xact(NULL),
+      display_predicate(_display_predicate),
+      only_predicate(_only_predicate), count(0),
+      last_entry(NULL), last_xact(NULL),
       totals_account(NULL, "<Total>"),
       only_collapse_if_zero(_only_collapse_if_zero) {
     TRACE_CTOR(collapse_xacts, "xact_handler_ptr");