From feff681f4464eb9b538bde613afaf3b03c7c223a Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 17:17:01 -0400 Subject: [PATCH 1/7] Improved argument parsing logic used by the REPL It now handles quoted strings, although it doesn't understand escape sequences yet. --- src/main.cc | 15 --------------- src/utils.cc | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.h | 2 ++ 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/main.cc b/src/main.cc index c9a922af..9f0e8690 100644 --- a/src/main.cc +++ b/src/main.cc @@ -37,21 +37,6 @@ using namespace ledger; -namespace { - strings_list split_arguments(char * line) - { - strings_list args; - - // jww (2009-02-04): This is too naive - for (char * p = std::strtok(line, " \t"); - p; - p = std::strtok(NULL, " \t")) - args.push_back(p); - - return args; - } -} - #ifdef HAVE_BOOST_PYTHON namespace ledger { extern char * argv0; diff --git a/src/utils.cc b/src/utils.cc index 0afea4c2..85a7aa46 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -442,6 +442,59 @@ string::~string() throw() { ledger::string empty_string(""); +ledger::strings_list split_arguments(const char * line) +{ + using namespace ledger; + + strings_list args; + + char buf[4096]; + char * q = buf; + char in_quoted_string = '\0'; + + for (const char * p = line; *p; p++) { + if (! in_quoted_string && std::isspace(*p)) { + if (q != buf) { + *q = '\0'; + args.push_back(buf); + q = buf; + } + } + else if (in_quoted_string != '\'' && *p == '\\') { + p++; + if (! *p) + throw_(std::logic_error, _("Invalid use of backslash")); + *q++ = *p; + } + else if (in_quoted_string != '"' && *p == '\'') { + if (in_quoted_string == '\'') + in_quoted_string = '\0'; + else + in_quoted_string = '\''; + } + else if (in_quoted_string != '\'' && *p == '"') { + if (in_quoted_string == '"') + in_quoted_string = '\0'; + else + in_quoted_string = '"'; + } + else { + *q++ = *p; + } + } + + if (in_quoted_string) + throw_(std::logic_error, + _("Unterminated string, expected '%1'") << in_quoted_string); + + if (q != buf) { + *q = '\0'; + args.push_back(buf); + } + + return args; +} + /********************************************************************** * * Logging diff --git a/src/utils.h b/src/utils.h index 7f5ca017..98bdf9af 100644 --- a/src/utils.h +++ b/src/utils.h @@ -237,6 +237,8 @@ inline bool operator!=(const string& __lhs, const char* __rhs) extern ledger::string empty_string; +ledger::strings_list split_arguments(const char * line); + #define IF_VERIFY() if (DO_VERIFY()) /*@}*/ From 8999607408c87624f3247a256974e6c341dd3611 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 17:17:12 -0400 Subject: [PATCH 2/7] If a pricing entry fails to parse, give an error --- src/textual.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/textual.cc b/src/textual.cc index 967e2f1b..f7b71429 100644 --- a/src/textual.cc +++ b/src/textual.cc @@ -461,7 +461,8 @@ void instance_t::price_xact_directive(char * line) { optional point = amount_t::current_pool->parse_price_directive(skip_ws(line + 1)); - assert(point); + if (! point) + throw parse_error(_("Pricing entry failed to parse")); } void instance_t::nomarket_directive(char * line) From 1ed22646f1349e2753ed1194f22dd9f028e8b638 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 17:17:23 -0400 Subject: [PATCH 3/7] Added an "echo" command, for REPL testing --- src/report.cc | 10 ++++++++++ src/report.h | 1 + 2 files changed, 11 insertions(+) diff --git a/src/report.cc b/src/report.cc index 9c8ad8ba..34231b5c 100644 --- a/src/report.cc +++ b/src/report.cc @@ -408,6 +408,14 @@ value_t report_t::reload_command(call_scope_t&) return true; } +value_t report_t::echo_command(call_scope_t& scope) +{ + interactive_t args(scope, "s"); + std::ostream& out(output_stream); + out << args.get(0) << std::endl; + return true; +} + bool report_t::maybe_import(const string& module) { if (lookup(string(OPT_PREFIX) + "import_")) { @@ -710,6 +718,8 @@ expr_t::ptr_op_t report_t::lookup(const string& name) else if (is_eq(q, "emacs")) return WRAP_FUNCTOR (reporter<>(new format_emacs_posts(output_stream), *this, "#emacs")); + else if (is_eq(q, "echo")) + return MAKE_FUNCTOR(report_t::echo_command); break; case 'p': diff --git a/src/report.h b/src/report.h index 5921a154..f0052128 100644 --- a/src/report.h +++ b/src/report.h @@ -178,6 +178,7 @@ public: } value_t reload_command(call_scope_t&); + value_t echo_command(call_scope_t& scope); keep_details_t what_to_keep() { bool lots = HANDLED(lots) || HANDLED(lots_actual); From d85a415bc5119d4271ca7355fe3e0ce3951c0d23 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 17:23:46 -0400 Subject: [PATCH 4/7] In the balance report, don't output any account twice --- src/output.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/output.cc b/src/output.cc index 92824714..fead2326 100644 --- a/src/output.cc +++ b/src/output.cc @@ -138,7 +138,8 @@ format_accounts::format_accounts(report_t& _report, std::size_t format_accounts::post_account(account_t& account) { - if (account.xdata().has_flags(ACCOUNT_EXT_TO_DISPLAY)) { + if (account.xdata().has_flags(ACCOUNT_EXT_TO_DISPLAY) && + ! account.xdata().has_flags(ACCOUNT_EXT_DISPLAYED)) { if (account.parent && account.parent->xdata().has_flags(ACCOUNT_EXT_TO_DISPLAY) && ! account.parent->xdata().has_flags(ACCOUNT_EXT_DISPLAYED)) From 151a8d87ee299b54da262346471aa71a729a6eb2 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 18:52:26 -0400 Subject: [PATCH 5/7] 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 | 66 +++++++++++++++++++++------------------------- src/compare.h | 7 ++++- src/iterators.cc | 68 +++++++++++++++++++++++++----------------------- src/iterators.h | 12 +++------ src/output.cc | 9 ++++--- src/output.h | 2 +- src/report.cc | 11 +++++--- 7 files changed, 88 insertions(+), 87 deletions(-) diff --git a/src/compare.cc b/src/compare.cc index cca22691..65e6a1e3 100644 --- a/src/compare.cc +++ b/src/compare.cc @@ -38,39 +38,29 @@ namespace ledger { -namespace { - template - void push_sort_value(std::list& 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 -void compare_items::find_sort_values(std::list& sort_values, - T * scope) +void push_sort_value(std::list& sort_values, + expr_t::ptr_op_t node, scope_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::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::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_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_values, T * scope); + void find_sort_values(std::list& 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,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(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(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() && 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 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 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 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(), From 4f11ded5bc3e1ae1b21a1d5513320a7e5c6a6c4b Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 18:52:43 -0400 Subject: [PATCH 6/7] Added t and T as valexpr synonyms t = display_amount, T = display_total --- src/report.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/report.cc b/src/report.cc index b15d9974..53426f39 100644 --- a/src/report.cc +++ b/src/report.cc @@ -903,6 +903,13 @@ expr_t::ptr_op_t report_t::lookup(const string& name) return MAKE_FUNCTOR(report_t::fn_total_expr); else if (is_eq(p, "today")) return MAKE_FUNCTOR(report_t::fn_today); + else if (is_eq(p, "t")) + return MAKE_FUNCTOR(report_t::fn_display_amount); + break; + + case 'T': + if (is_eq(p, "T")) + return MAKE_FUNCTOR(report_t::fn_display_total); break; case 'u': From 3fdd75fb5b2614f7dab29fd5ad5c9efed6e80b79 Mon Sep 17 00:00:00 2001 From: John Wiegley Date: Mon, 26 Oct 2009 19:08:15 -0400 Subject: [PATCH 7/7] Balance assertions now really assert There are two kinds of balance related options for a posting: a balance assignment, where the amount of the posting is blank and so it fills it in to make the assertion true; and plain assertions, where the amount is not blank and an error is reported if the balance does not match the given amount after the posting is taken into account. --- src/textual.cc | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/textual.cc b/src/textual.cc index f7b71429..56c67ef3 100644 --- a/src/textual.cc +++ b/src/textual.cc @@ -970,8 +970,12 @@ post_t * instance_t::parse_post(char * line, static_cast(expr_t::PARSE_SINGLE) | static_cast(expr_t::PARSE_NO_MIGRATE)); - if (post->assigned_amount->is_null()) - throw parse_error(_("An assigned balance must evaluate to a constant value")); + if (post->assigned_amount->is_null()) { + if (post->amount.is_null()) + throw parse_error(_("Balance assignment must evaluate to a constant")); + else + throw parse_error(_("Balance assertion must evaluate to a constant")); + } DEBUG("textual.parse", "line " << linenum << ": " << "POST assign: parsed amt = " << *post->assigned_amount); @@ -980,46 +984,50 @@ post_t * instance_t::parse_post(char * line, value_t account_total(post->account->self_total(false) .strip_annotations(keep_details_t())); - DEBUG("post.assign", "line " << linenum << ": " - "account balance = " << account_total); - DEBUG("post.assign", "line " << linenum << ": " - "post amount = " << amt); + DEBUG("post.assign", + "line " << linenum << ": " "account balance = " << account_total); + DEBUG("post.assign", + "line " << linenum << ": " "post amount = " << amt); - amount_t diff; + amount_t diff = amt; switch (account_total.type()) { case value_t::AMOUNT: - diff = amt - account_total.as_amount(); + diff -= account_total.as_amount(); break; case value_t::BALANCE: if (optional comm_bal = account_total.as_balance().commodity_amount(amt.commodity())) - diff = amt - *comm_bal; - else - diff = amt; + diff -= *comm_bal; break; default: - diff = amt; break; } - DEBUG("post.assign", "line " << linenum << ": " - << "diff = " << diff); - DEBUG("textual.parse", "line " << linenum << ": " - << "POST assign: diff = " << diff); + DEBUG("post.assign", + "line " << linenum << ": " << "diff = " << diff); + DEBUG("textual.parse", + "line " << linenum << ": " << "POST assign: diff = " << diff); if (! diff.is_zero()) { if (! post->amount.is_null()) { diff -= post->amount; if (! diff.is_zero()) { +#if 1 + throw_(parse_error, _("Balance assertion off by %1") << diff); +#else + // This code, rather than issuing an error if a balance assignment + // fails, creates a balancing transaction that causes the + // assertion to be true. post_t * temp = new post_t(post->account, diff, ITEM_GENERATED | POST_CALCULATED); xact->add_post(temp); DEBUG("textual.parse", "line " << linenum << ": " << "Created balancing posting"); +#endif } } else { post->amount = diff; @@ -1033,7 +1041,7 @@ post_t * instance_t::parse_post(char * line, else next = skip_ws(p + static_cast(stream.tellg())); } else { - throw parse_error(_("Expected an assigned balance amount")); + throw parse_error(_("Expected an balance assignment/assertion amount")); } }