hledger icon indicating copy to clipboard operation
hledger copied to clipboard

Amount expressions, multi-commodity postings

Open ag-eitilt opened this issue 5 years ago • 12 comments

This PR comes out of #913, which was closed as it both was only a small change (fixing before-unnoticed undefined behaviour) and was written in a way that was oriented more toward (this) future work than toward simply fixing that bug. Besides the larger scope here eclipsing that minor fix, some unclean commits and unfortunate naming resulted in the discussion there becoming confused; while the latter in itself would not be enough to close the earlier PR, I felt that taking it along with the former resulted in it making more sense to focus the conversation on the features introduced in later commits.

On that topic, this adds the ability to write postings usiing basic arithmatic operations inline, implementing #183 and #923, in a way that isn't hopelessly entangled with cruft, WIP code, and other features (as my first attempt #871 was). To separate out all its effects:

  • Posting amounts and assertations/assignments may be written as multiple amounts (whether of the same commodity or not) connected via addition or subtraction: $1 + $2
    • If multiple amounts of the same commodity are given, they are condensed into a single value: the above is equivalent to $3
    • If amounts of different commodities are given, they're equivalent to having put the individual amounts on separate lines, except that any transaction modifiers applied to that posting will only add static values once: = a ... *1 + $2 applied to a $3 + 4 CAD results in $3 + 4 CAD not $5 + 4 CAD
  • Commodities using parentheses now must be quoted: 1 "(XYZ)"
  • Standard postings are now no longer permitted to begin with a modifier-style multiplier

And, non-user-facing:

  • A Posting's amount is stored as a MixedAmount rather than a simple Amount
  • Amount no longer has the field amultiplier; its role is taken over by storing an Amount object in the new field Posting.pmultiplier
  • Hledger.Read.Common exports a new parser mamountp handling the amount expression logic
  • It also exports modifierp which functions mostly the same as amountp, except that its values are preceded by a * and never assigned the default commodity
  • The internal function multiplierp (previously only checking the presence of a single character) was rewritten to return the value of the (unitless) multiplier(s) and/or divisior(s) it matched
  • postingsp, postingp, and amountwithoutpricep now take a Bool argument indicating whether they're being parsed from within a TransactionModifier

ag-eitilt avatar Dec 07 '18 22:12 ag-eitilt

The Travis failiure isn't in any of my code, so I'm not entirely sure why it's only appeared for this push. Maybe one of the dependencies updated in a fragile way?

ag-eitilt avatar Dec 07 '18 22:12 ag-eitilt

Thanks for the PR.

Interesting travis failure.. it appears to building filemanip, and failing. It might or might not have anything to do with https://github.com/commercialhaskell/stackage/pull/4206 . hledger is supposed to be using Glob, not filemanip, I don't know if something else is depending on filemanip.

simonmichael avatar Dec 08 '18 00:12 simonmichael

Actual test failures this time! Strangely, though, they're not showing up with my local copy. I'll take a look at that when I get the chance.

ag-eitilt avatar Dec 14 '18 20:12 ag-eitilt

Sorry for the lack of input.. it may take me a little longer to get back to this.

simonmichael avatar Dec 15 '18 01:12 simonmichael

Part of the reason for delay: I already feel hledger's complexity is too hard to keep up with, and I can't assume any serious increase in our maintenance capacity, so I fear the complexity and cost this PR may bring and I'm almost anti-motivated to merge it, especially as I don't yet see or feel any pressing need for it.

So, possible next steps:

  • rebase it against master - I have fixed the failing roi tests
  • I and hopefully others will install and play with it. Often that will change my mood.
  • let's spend some time using and refining it - as your fork if necessary - and gather a bunch real world examples where it's useful

simonmichael avatar Jan 05 '19 18:01 simonmichael

Hi @ag-eitilt , will you have time to rebase this against latest master any time soon ?

simonmichael avatar Feb 15 '19 18:02 simonmichael

(If not, let me know and I might try.)

simonmichael avatar Feb 16 '19 18:02 simonmichael

I may have a use case, but I'm not sure this PR supports it. If you think it does, I can try it and report:

~ 2018  residency rules
    Cyprus      60 days ; minimum days to spend in Cyprus
    Germany     90 days ; maximum days to spend in Germany 
    France      MAX(Cyprus,Germany) - 1 days ; maximum days to spend in France

Note that the France rule refers to the current value of Cyprus and Germany, not the value from the automated rules above it. I think my use case is too contrived, but I thought I might as well mention it, since I put more and more in hledger these days

lestephane avatar Nov 07 '19 07:11 lestephane

My usecase for this feature is the semi-automated splitting of expenses, something to support the transactions of the form

2019/12/12 Restaurant
    Assets    -100
    Expenses   100/2
    Liabilities:SomeOtherPerson  100/2

The support for fractional (not necessarily decimal) amounts would be great, too. Another recent usecase which lead me to finding this PR, is "three people chip in to buy two gifts; the total sum is divisible by three, but individual prices are not divisible by three". Either I lose granularity of transactions I want and dump everything in one posting (don't really want to), or deal with rounding errors that I need to manually fix later (which leads to ugly postings and balances).

ShrykeWindgrace avatar Dec 16 '19 14:12 ShrykeWindgrace

I am trying to bring this PR up to date with the current master branch and got confused during rebasing due to some of the work being duplicated in master, e.g. 7ed2a0aa9b3c2863f7a1d5df7da7d0404f0edc54 What is the purpose of allowCommodityMult argument in postingp?

l29ah avatar Nov 06 '23 01:11 l29ah

I can't find any such name

simonmichael avatar Nov 06 '23 06:11 simonmichael

Ah, in this PR at https://github.com/simonmichael/hledger/pull/934/files#diff-7f7e2a1fff079623a40926984dc95d7de0fac5a9575881e8ec09ce309903a574R583 . I'd guess it's to do with https://hledger.org/dev/hledger.html#auto-postings > "a numeric multiplier".

simonmichael avatar Nov 06 '23 06:11 simonmichael