Fixed sorting in bal reports when --flat is used

Note that sorting on the "total" is not the same thing as sorting on the
"display_total" when multiple commodities are in use and the -X flag is
selected!  One should always sort on display_total, since that's the
value which is shown in the report.  'T' is a synonym for display_total.

7 files changed, 81 insertions, 80 deletions

diff --git a/src/compare.cc b/src/compare.cc
index cca22691..65e6a1e3 100644
--- a/src/compare.cc
+++ b/src/compare.cc
@@ -38,41 +38,31 @@
 
 namespace ledger {
 
-namespace {
-  template <typename T>
-  void push_sort_value(std::list<sort_value_t>& sort_values,
-		       expr_t::ptr_op_t node, T * scope)
-  {
-    if (node->kind == expr_t::op_t::O_CONS) {
-      push_sort_value(sort_values, node->left(), scope);
-      push_sort_value(sort_values, node->right(), scope);
-    }
-    else {
-      bool inverted = false;
+void push_sort_value(std::list<sort_value_t>& sort_values,
+		     expr_t::ptr_op_t node, scope_t& scope)
+{
+  if (node->kind == expr_t::op_t::O_CONS) {
+    push_sort_value(sort_values, node->left(), scope);
+    push_sort_value(sort_values, node->right(), scope);
+  }
+  else {
+    bool inverted = false;
 
-      if (node->kind == expr_t::op_t::O_NEG) {
-	inverted = true;
-	node = node->left();
-      }
+    if (node->kind == expr_t::op_t::O_NEG) {
+      inverted = true;
+      node = node->left();
+    }
 
-      sort_values.push_back(sort_value_t());
-      sort_values.back().inverted = inverted;
-      sort_values.back().value	  = expr_t(node).calc(*scope).simplified();
+    sort_values.push_back(sort_value_t());
+    sort_values.back().inverted = inverted;
+    sort_values.back().value	= expr_t(node).calc(scope).simplified();
 
-      if (sort_values.back().value.is_null())
-	throw_(calc_error,
-	       _("Could not determine sorting value based an expression"));
-    }
+    if (sort_values.back().value.is_null())
+      throw_(calc_error,
+	     _("Could not determine sorting value based an expression"));
   }
 }
 
-template <typename T>
-void compare_items<T>::find_sort_values(std::list<sort_value_t>& sort_values,
-					T * scope)
-{
-  push_sort_value(sort_values, sort_order.get_op(), scope);
-}
-
 template <>
 bool compare_items<post_t>::operator()(post_t * left, post_t * right)
 {
@@ -81,13 +71,15 @@ bool compare_items<post_t>::operator()(post_t * left, post_t * right)
 
   post_t::xdata_t& lxdata(left->xdata());
   if (! lxdata.has_flags(POST_EXT_SORT_CALC)) {
-    find_sort_values(lxdata.sort_values, left);
+    bind_scope_t bound_scope(*sort_order.get_context(), *left);
+    find_sort_values(lxdata.sort_values, bound_scope);
     lxdata.add_flags(POST_EXT_SORT_CALC);
   }
 
   post_t::xdata_t& rxdata(right->xdata());
   if (! rxdata.has_flags(POST_EXT_SORT_CALC)) {
-    find_sort_values(rxdata.sort_values, right);
+    bind_scope_t bound_scope(*sort_order.get_context(), *right);
+    find_sort_values(rxdata.sort_values, bound_scope);
     rxdata.add_flags(POST_EXT_SORT_CALC);
   }
 
@@ -102,13 +94,15 @@ bool compare_items<account_t>::operator()(account_t * left, account_t * right)
 
   account_t::xdata_t& lxdata(left->xdata());
   if (! lxdata.has_flags(ACCOUNT_EXT_SORT_CALC)) {
-    find_sort_values(lxdata.sort_values, left);
+    bind_scope_t bound_scope(*sort_order.get_context(), *left);
+    find_sort_values(lxdata.sort_values, bound_scope);
     lxdata.add_flags(ACCOUNT_EXT_SORT_CALC);
   }
 
   account_t::xdata_t& rxdata(right->xdata());
   if (! rxdata.has_flags(ACCOUNT_EXT_SORT_CALC)) {
-    find_sort_values(rxdata.sort_values, right);
+    bind_scope_t bound_scope(*sort_order.get_context(), *right);
+    find_sort_values(rxdata.sort_values, bound_scope);
     rxdata.add_flags(ACCOUNT_EXT_SORT_CALC);
   }
 
diff --git a/src/compare.h b/src/compare.h
index e1207bc3..c0a72327 100644
--- a/src/compare.h
+++ b/src/compare.h
@@ -53,6 +53,9 @@ namespace ledger {
 class post_t;
 class account_t;
 
+void push_sort_value(std::list<sort_value_t>& sort_values,
+		     expr_t::ptr_op_t node, scope_t& scope);
+
 /**
  * @brief Brief
  *
@@ -76,7 +79,9 @@ public:
     TRACE_DTOR(compare_items);
   }
 
-  void find_sort_values(std::list<sort_value_t>& sort_values, T * scope);
+  void find_sort_values(std::list<sort_value_t>& sort_values, scope_t& scope) {
+    push_sort_value(sort_values, sort_order.get_op(), scope);
+  }
 
   bool operator()(T * left, T * right);
 };
diff --git a/src/iterators.cc b/src/iterators.cc
index 63cbb9b2..020f70b0 100644
--- a/src/iterators.cc
+++ b/src/iterators.cc
@@ -173,46 +173,22 @@ account_t * basic_accounts_iterator::operator()()
   return account;
 }
 
-void sorted_accounts_iterator::sort_accounts(account_t& account,
-					     accounts_deque_t& deque)
-{
-  foreach (accounts_map::value_type& pair, account.accounts)
-    deque.push_back(pair.second);
-
-  std::stable_sort(deque.begin(), deque.end(),
-		   compare_items<account_t>(sort_cmp));
-
-#if defined(DEBUG_ON)
-  if (SHOW_DEBUG("accounts.sorted")) {
-    foreach (account_t * account, deque)
-      DEBUG("accounts.sorted", "Account: " << account->fullname());
-  }
-#endif
-}
-
-void sorted_accounts_iterator::push_all(account_t& account)
-{
-  accounts_deque_t& deque(accounts_list.back());
-
-  foreach (accounts_map::value_type& pair, account.accounts) {
-    deque.push_back(pair.second);
-    push_all(*pair.second);
-  }
-}
-
 void sorted_accounts_iterator::push_back(account_t& account)
 {
   accounts_list.push_back(accounts_deque_t());
 
   if (flatten_all) {
-    push_all(account);
+    push_all(account, accounts_list.back());
+
     std::stable_sort(accounts_list.back().begin(),
 		     accounts_list.back().end(),
 		     compare_items<account_t>(sort_cmp));
+
 #if defined(DEBUG_ON)
     if (SHOW_DEBUG("accounts.sorted")) {
       foreach (account_t * account, accounts_list.back())
-	DEBUG("accounts.sorted", "Account: " << account->fullname());
+	DEBUG("accounts.sorted",
+	      "Account (flat): " << account->fullname());
     }
 #endif
   } else {
@@ -223,6 +199,32 @@ void sorted_accounts_iterator::push_back(account_t& account)
   sorted_accounts_end.push_back(accounts_list.back().end());
 }
 
+void sorted_accounts_iterator::push_all(account_t& account,
+					accounts_deque_t& deque)
+{
+  foreach (accounts_map::value_type& pair, account.accounts) {
+    deque.push_back(pair.second);
+    push_all(*pair.second, deque);
+  }
+}
+
+void sorted_accounts_iterator::sort_accounts(account_t& account,
+					     accounts_deque_t& deque)
+{
+  foreach (accounts_map::value_type& pair, account.accounts)
+    deque.push_back(pair.second);
+
+  std::stable_sort(deque.begin(), deque.end(),
+		   compare_items<account_t>(sort_cmp));
+
+#if defined(DEBUG_ON)
+  if (SHOW_DEBUG("accounts.sorted")) {
+    foreach (account_t * account, deque)
+      DEBUG("accounts.sorted", "Account: " << account->fullname());
+  }
+#endif
+}
+
 account_t * sorted_accounts_iterator::operator()()
 {
   while (! sorted_accounts_i.empty() &&
diff --git a/src/iterators.h b/src/iterators.h
index ae2ddaf9..a1563539 100644
--- a/src/iterators.h
+++ b/src/iterators.h
@@ -255,12 +255,8 @@ class sorted_accounts_iterator : public accounts_iterator
   std::list<accounts_deque_t::const_iterator> sorted_accounts_end;
 
 public:
-  sorted_accounts_iterator(const expr_t& _sort_cmp, bool _flatten_all)
-    : sort_cmp(_sort_cmp), flatten_all(_flatten_all) {
-    TRACE_CTOR(sorted_accounts_iterator, "const expr_t&, bool");
-  }
-  sorted_accounts_iterator(const expr_t& _sort_cmp, bool _flatten_all,
-			   account_t& account)
+  sorted_accounts_iterator(account_t& account,
+			   const expr_t& _sort_cmp, bool _flatten_all)
     : sort_cmp(_sort_cmp), flatten_all(_flatten_all) {
     TRACE_CTOR(sorted_accounts_iterator, "const expr_t&, bool, account_t&");
     push_back(account);
@@ -269,9 +265,9 @@ public:
     TRACE_DTOR(sorted_accounts_iterator);
   }
 
-  void sort_accounts(account_t& account, accounts_deque_t& deque);
-  void push_all(account_t& account);
   void push_back(account_t& account);
+  void push_all(account_t& account, accounts_deque_t& deque);
+  void sort_accounts(account_t& account, accounts_deque_t& deque);
 
   virtual account_t * operator()();
 };
diff --git a/src/output.cc b/src/output.cc
index fead2326..a0c2581b 100644
--- a/src/output.cc
+++ b/src/output.cc
@@ -136,14 +136,14 @@ format_accounts::format_accounts(report_t&     _report,
   }
 }
 
-std::size_t format_accounts::post_account(account_t& account)
+std::size_t format_accounts::post_account(account_t& account, const bool flat)
 {
   if (account.xdata().has_flags(ACCOUNT_EXT_TO_DISPLAY) &&
       ! account.xdata().has_flags(ACCOUNT_EXT_DISPLAYED)) {
-    if (account.parent &&
+    if (! flat && account.parent &&
 	account.parent->xdata().has_flags(ACCOUNT_EXT_TO_DISPLAY) &&
 	! account.parent->xdata().has_flags(ACCOUNT_EXT_DISPLAYED))
-      post_account(*account.parent);
+      post_account(*account.parent, flat);
 
     account.xdata().add_flags(ACCOUNT_EXT_DISPLAYED);
 
@@ -208,7 +208,7 @@ void format_accounts::flush()
   std::size_t displayed = 0;
 
   foreach (account_t * account, posted_accounts)
-    displayed += post_account(*account);
+    displayed += post_account(*account, report.HANDLED(flat));
 
   if (displayed > 1 &&
       ! report.HANDLED(no_total) && ! report.HANDLED(percent)) {
@@ -222,6 +222,7 @@ void format_accounts::flush()
 
 void format_accounts::operator()(account_t& account)
 {
+  DEBUG("account.display", "Posting account: " << account.fullname());
   posted_accounts.push_back(&account);
 }
 
diff --git a/src/output.h b/src/output.h
index a35d81cd..5e06db9a 100644
--- a/src/output.h
+++ b/src/output.h
@@ -109,7 +109,7 @@ public:
   std::pair<std::size_t, std::size_t>
   mark_accounts(account_t& account, const bool flat);
 
-  virtual std::size_t post_account(account_t& account);
+  virtual std::size_t post_account(account_t& account, const bool flat);
   virtual void	      flush();
 
   virtual void operator()(account_t& account);
diff --git a/src/report.cc b/src/report.cc
index 34231b5c..b15d9974 100644
--- a/src/report.cc
+++ b/src/report.cc
@@ -81,11 +81,14 @@ void report_t::accounts_report(acct_handler_ptr handler)
 				      true), walker);
 
   scoped_ptr<accounts_iterator> iter;
-  if (! HANDLED(sort_))
+  if (! HANDLED(sort_)) {
     iter.reset(new basic_accounts_iterator(*session.master));
-  else
-    iter.reset(new sorted_accounts_iterator(HANDLER(sort_).str(),
-					    HANDLED(flat), *session.master.get()));
+  } else {
+    expr_t sort_expr(HANDLER(sort_).str());
+    sort_expr.set_context(this);
+    iter.reset(new sorted_accounts_iterator(*session.master.get(),
+					    sort_expr, HANDLED(flat)));
+  }
 
   if (HANDLED(display_))
     pass_down_accounts(handler, *iter.get(),