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.

src/compare.cc

@@ -38,39 +38,29 @@
 
 namespace ledger {
 
-namespace {
+void push_sort_value(std::list<sort_value_t>& sort_values,
-  template <typename T>
+		     expr_t::ptr_op_t node, scope_t& scope)
-  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;
-
-      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();
-
-      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);
+  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();
+    }
+
+    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"));
+  }
 }
 
 template <>
@@ -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);
   }
 

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);
 };

src/iterators.cc

@@ -173,6 +173,41 @@ account_t * basic_accounts_iterator::operator()()
   return account;
 }
 
+void sorted_accounts_iterator::push_back(account_t& account)
+{
+  accounts_list.push_back(accounts_deque_t());
+
+  if (flatten_all) {
+    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 (flat): " << account->fullname());
+    }
+#endif
+  } else {
+    sort_accounts(account, accounts_list.back());
+  }
+
+  sorted_accounts_i.push_back(accounts_list.back().begin());
+  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)
 {
@@ -190,39 +225,6 @@ void sorted_accounts_iterator::sort_accounts(account_t& account,
 #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);
-    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());
-    }
-#endif
-  } else {
-    sort_accounts(account, accounts_list.back());
-  }
-
-  sorted_accounts_i.push_back(accounts_list.back().begin());
-  sorted_accounts_end.push_back(accounts_list.back().end());
-}
-
 account_t * sorted_accounts_iterator::operator()()
 {
   while (! sorted_accounts_i.empty() &&

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)
+  sorted_accounts_iterator(account_t& account,
-    : sort_cmp(_sort_cmp), flatten_all(_flatten_all) {
+			   const expr_t& _sort_cmp, bool _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)
     : 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()();
 };

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);
 }
 

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);

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
+  } else {
-    iter.reset(new sorted_accounts_iterator(HANDLER(sort_).str(),
+    expr_t sort_expr(HANDLER(sort_).str());
-					    HANDLED(flat), *session.master.get()));
+    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(),