Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What does the expr argument do in account_t::amount? #2273

Open
dbear496 opened this issue Jul 14, 2023 · 0 comments · May be fixed by #2274
Open

What does the expr argument do in account_t::amount? #2273

dbear496 opened this issue Jul 14, 2023 · 0 comments · May be fixed by #2274

Comments

@dbear496
Copy link
Contributor

I am thinking about adding a feature that would involve getting an account balance based on a filtered set of postings. I came across the account_t::amount method which takes a expr_t parameter. It appears that this expr_t will modify a posting amount before it is added to the account total. However, it does not appear this would jive well with the way the account total is computed, that is, by updating a previously computed account total. So I believe this causes two issues:

  1. The account_t::amount method, when called with an expr argument, will not compute the entire account total based on expr. Only postings not included since the most recent call to account_t::amount will reflect expr. This could lead to unexpected results if a user makes use of the expr argument.
  2. The amount method could give unexpected results if a previous invocation of account_t::amount used the expr argument as some postings may be factored into the total with the old expr.

Is this intended behavior? Or is there some safeguard to prevent this that I missed? It is difficult for me to know whether this is intended since I cannot find any call to account_t::amount that uses the expr argument.

If the current behavior is not intended, then I believe a fix would be relatively easy: whenever the expr argument exists, recompute the total from scratch and do not affect the totals stored in xdata_->self_details.

dbear496 added a commit to dbear496/ledger-cli that referenced this issue Jul 14, 2023
@dbear496 dbear496 linked a pull request Jul 14, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant