Fixed sorting of equity output

src/balance.cc

@@ -240,17 +240,8 @@ balance_t::strip_annotations(const keep_details_t& what_to_keep) const
   return temp;
 }
 
-void balance_t::print(std::ostream&       out,
+void balance_t::map_sorted_amounts(function<void(const amount_t&)> fn) const
-                      const int           first_width,
-                      const int           latter_width,
-                      const uint_least8_t flags) const
 {
-  bool first  = true;
-  int  lwidth = latter_width;
-
-  if (lwidth == -1)
-    lwidth = first_width;
-
   typedef std::vector<const amount_t *> amounts_array;
   amounts_array sorted;
 
@@ -261,30 +252,79 @@ void balance_t::print(std::ostream&       out,
   std::stable_sort(sorted.begin(), sorted.end(),
                    commodity_t::compare_by_commodity());
 
-  foreach (const amount_t * amount, sorted) {
+  foreach (const amount_t * amount, sorted)
-    int width;
+    fn(*amount);
-    if (! first) {
+}
-      out << std::endl;
+
-      width = lwidth;
+namespace {
-    } else {
+  struct print_amount_from_balance
-      first = false;
+  {
-      width = first_width;
+    std::ostream& out;
+    bool&         first;
+    int           fwidth;
+    int           lwidth;
+    uint_least8_t flags;
+
+    explicit print_amount_from_balance(std::ostream& _out,
+                                       bool& _first,
+                                       int _fwidth, int _lwidth,
+                                       uint_least8_t _flags)
+      : out(_out), first(_first), fwidth(_fwidth), lwidth(_lwidth),
+        flags(_flags) {
+      TRACE_CTOR(print_amount_from_balance,
+                 "ostream&, int, int, uint_least8_t");
+    }
+    print_amount_from_balance(const print_amount_from_balance& other)
+      : out(other.out), first(other.first), fwidth(other.fwidth),
+        lwidth(other.lwidth), flags(other.flags) {
+      TRACE_CTOR(print_amount_from_balance, "copy");
+    }
+    ~print_amount_from_balance() throw() {
+      TRACE_DTOR(print_amount_from_balance);
     }
 
-    std::ostringstream buf;
+    void operator()(const amount_t& amount) {
-    amount->print(buf, flags);
+      int width;
-    justify(out, buf.str(), width, flags & AMOUNT_PRINT_RIGHT_JUSTIFY,
+      if (! first) {
-            flags & AMOUNT_PRINT_COLORIZE && amount->sign() < 0);
+        out << std::endl;
-  }
+        width = lwidth;
+      } else {
+        first = false;
+        width = fwidth;
+      }
 
-  if (first) {
+      std::ostringstream buf;
-    out.width(first_width);
+      amount.print(buf, flags);
-    if (flags & AMOUNT_PRINT_RIGHT_JUSTIFY)
+
-      out << std::right;
+      justify(out, buf.str(), width,
-    else
+              flags & AMOUNT_PRINT_RIGHT_JUSTIFY,
-      out << std::left;
+              flags & AMOUNT_PRINT_COLORIZE && amount.sign() < 0);
-    out << 0;
+    }
-  }
+
+    void close() {
+      out.width(fwidth);
+      if (flags & AMOUNT_PRINT_RIGHT_JUSTIFY)
+        out << std::right;
+      else
+        out << std::left;
+      out << 0;
+    }
+  };
+}
+
+void balance_t::print(std::ostream&       out,
+                      const int           first_width,
+                      const int           latter_width,
+                      const uint_least8_t flags) const
+{
+  bool first = true;
+  print_amount_from_balance
+    amount_printer(out, first, first_width,
+                   latter_width == 1 ? first_width : latter_width, flags);
+  map_sorted_amounts(amount_printer);
+
+  if (first)
+    amount_printer.close();
 }
 
 void to_xml(std::ostream& out, const balance_t& bal)

src/balance.h

@@ -506,6 +506,14 @@ public:
    */
   balance_t strip_annotations(const keep_details_t& what_to_keep) const;
 
+  /**
+   * Iteration primitives.  `map_sorted_amounts' allows one to visit
+   * each amount in balance in the proper order for displaying to the
+   * user.  Mostly used by `print' and other routinse where the sort
+   * order of the amounts' commodities is significant.
+   */
+  void map_sorted_amounts(function<void(const amount_t&)> fn) const;
+
   /**
    * Printing methods.  A balance may be output to a stream using the
    * `print' method.  There is also a global operator<< defined which

src/filters.cc

@@ -1030,6 +1030,40 @@ void interval_posts::flush()
   subtotal_posts::flush();
 }
 
+namespace {
+  struct create_post_from_amount
+  {
+    post_handler_ptr handler;
+    xact_t&          xact;
+    account_t&       balance_account;
+    temporaries_t&   temps;
+
+    explicit create_post_from_amount(post_handler_ptr _handler,
+                                     xact_t&          _xact,
+                                     account_t&       _balance_account,
+                                     temporaries_t&   _temps)
+      : handler(_handler), xact(_xact),
+        balance_account(_balance_account), temps(_temps) {
+      TRACE_CTOR(create_post_from_amount,
+                 "post_handler_ptr, xact_t&, account_t&, temporaries_t&");
+    }
+    create_post_from_amount(const create_post_from_amount& other)
+      : handler(other.handler), xact(other.xact),
+        balance_account(other.balance_account), temps(other.temps) {
+      TRACE_CTOR(create_post_from_amount, "copy");
+    }
+    ~create_post_from_amount() throw() {
+      TRACE_DTOR(create_post_from_amount);
+    }
+
+    void operator()(const amount_t& amount) {
+      post_t& balance_post = temps.create_post(xact, &balance_account);
+      balance_post.amount = - amount;
+      (*handler)(balance_post);
+    }      
+  };
+}
+
 void posts_as_equity::report_subtotal()
 {
   date_t finish;
@@ -1076,28 +1110,18 @@ void posts_as_equity::report_subtotal()
   }
   values.clear();
 
-#if 1
   // This last part isn't really needed, since an Equity:Opening
   // Balances posting with a null amount will automatically balance with
   // all the other postings generated.  But it does make the full
   // balancing amount clearer to the user.
   if (! total.is_zero()) {
-    if (total.is_balance()) {
+    create_post_from_amount post_creator(handler, xact,
-      foreach (const balance_t::amounts_map::value_type& pair,
+                                         *balance_account, temps);
-               total.as_balance().amounts) {
+    if (total.is_balance())
-        if (! pair.second.is_zero()) {
+      total.as_balance_lval().map_sorted_amounts(post_creator);
-          post_t& balance_post = temps.create_post(xact, balance_account);
+    else
-          balance_post.amount = - pair.second;
+      post_creator(total.to_amount());
-          (*handler)(balance_post);
-        }
-      }
-    } else {
-      post_t& balance_post = temps.create_post(xact, balance_account);
-      balance_post.amount = - total.to_amount();
-      (*handler)(balance_post);
-    }
   }
-#endif
 }
 
 void by_payee_posts::flush()

test/baseline/opt-equity.test

@@ -41,9 +41,9 @@ test equity --lot-prices
     Assets:Bank                            -3.80 GBP
     Assets:Broker                       2 AAA {0.90 GBP}
     Assets:Broker                       2 AAA {1.00 GBP}
-    Equity:Opening Balances                 3.80 GBP
     Equity:Opening Balances             -2 AAA {0.90 GBP}
     Equity:Opening Balances             -2 AAA {1.00 GBP}
+    Equity:Opening Balances                 3.80 GBP
 end test
 
 test equity --lots
@@ -51,8 +51,8 @@ test equity --lots
     Assets:Bank                            -3.80 GBP
     Assets:Broker                       2 AAA {0.90 GBP} [2011/03/04]
     Assets:Broker                       2 AAA {1.00 GBP} [2011/03/05]
-    Equity:Opening Balances                 3.80 GBP
     Equity:Opening Balances             -2 AAA {0.90 GBP} [2011/03/04]
     Equity:Opening Balances             -2 AAA {1.00 GBP} [2011/03/05]
+    Equity:Opening Balances                 3.80 GBP
 end test