Amount expressions, multi-commodity postings by ag-eitilt · Pull Request #934 · simonmichael/hledger

ag-eitilt · Dec 7, 2018

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 · Dec 7, 2018

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?

simonmichael · Dec 8, 2018

Thanks for the PR.

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

simonmichael · Dec 8, 2018

Terrific docs!

simonmichael · Dec 8, 2018

This feature feels like an unnecessary complication at first glance - perhaps first PR should focus just on adding amount expressions, which is a big change.

I see where you're coming from, but I'm not sure whether people will find it more confusing to have amount expressions in transaction modifiers except when they're multipliers, or to have transaction modifiers doing two things at once. The latter felt a bit more natural to me, obviously, but thinking about it I do tend to have a more multi-track mind than most (though with most users being people on Linux using the console...) It's not a functional difference from that side since even if we do split things, users can just put the multiplier and the expression on separate lines.

Basically, I see more people getting confused if they're able to write ($1 + $2) * 3 + $4 but not apply x * 3 + $4 to ($1 + $2) than I see people getting thrown off by both x * 3 and y + $4 being on one line.

From the technical side, this is essentially just a user-facing interface to the split between pamount and pmultiplier; I'm not sure we really want to merge the two into an Either since we'd then need to handle all the corner cases (in printing, say) where a posting has a Quantity instead of an Amount. Without multipliers + expressions in journals, though, there's not as much impetus keeping them apart, and no way of using shelltest to test the behaviour when both are present.

Just to clarify, that's not shutting down my going back and not putting that in, it's just my argument for why I think it belongs. If you do think it should go, than I'm happy enough cutting it.

This does break journals with standard transactions including multipliers (shouldn't be used in reality).

ag-eitilt · Dec 14, 2018

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.

simonmichael · Dec 15, 2018

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

simonmichael · Jan 5, 2019

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 · Feb 15, 2019

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

simonmichael · Feb 16, 2019

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

lestephane · Nov 7, 2019

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

ShrykeWindgrace · Dec 16, 2019

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

l29ah · Nov 6, 2023

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. 7ed2a0a
What is the purpose of allowCommodityMult argument in postingp?

simonmichael · Nov 6, 2023

I can't find any such name

simonmichael · Nov 6, 2023

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 · May 17, 2025

Hi @ag-eitilt, @l29ah, all - are you interested in updating this hledger PR: Amount expressions, multi-commodity postings ?

I'm still on the fence about the cost/benefit, how about you ?

l29ah · May 17, 2025

I'm still interested in the functionality, but i failed to understand the code last time i tried, and ended up doing a workaround for a ledger-cli bug instead, and use it now.

simonmichael · May 19, 2025

@l29ah thanks. I guess I'll close this for now, we can reopen when someone wants to work on it.

ag-eitilt mentioned this pull request Dec 7, 2018

Separate multiplier from Amount #913

Closed

simonmichael reviewed Dec 8, 2018

View reviewed changes

simonmichael mentioned this pull request Dec 11, 2018

CSV output if using more than 1 currency not very useful #238

Closed

ag-eitilt added 5 commits December 14, 2018 11:05

lib: Separate multiplier type from Amount

af965e6

This does break journals with standard transactions including multipliers (shouldn't be used in reality).

lib: Let balance assertions support MixedAmounts

fba0796

lib: journal: Add basic arithmetic

11830eb

lib: journal: Add scalar multiplication and division

c7450f6

docs: Document amount expressions

4d7d06b

ag-eitilt force-pushed the expressions branch from 8d90217 to 4d7d06b Compare December 14, 2018 19:05

simonmichael added the needs-rebase To unblock: needs to be rebased against latest master branch label Jan 5, 2019

simonmichael removed the needs-review To unblock: needs more code/docs/design review by someone label Jan 5, 2019

simonmichael changed the title ~~Amount expressions~~ Amount expressions, multi-commodity postings Jan 7, 2019

simonmichael mentioned this pull request Feb 13, 2019

Support assertions which include subaccounts #290

Closed

simonmichael force-pushed the master branch from 0ae9583 to 0bc16d4 Compare February 21, 2019 04:24

simonmichael removed the needs-value-proposition To unblock: needs clearer justification, review of benefits vs costs label Mar 15, 2019

simonmichael added the needs-testing To unblock: needs more developer testing or general usage label Jul 15, 2019

simonmichael mentioned this pull request Sep 28, 2019

Rounding in auto posting to the nearest cent, to calculate VAT #1092

Open

simonmichael force-pushed the master branch from abcbb30 to 64470ba Compare December 28, 2019 21:33

simonmichael removed the help wanted label Mar 7, 2020

simonmichael mentioned this pull request Apr 12, 2021

Hledger can't parse valid Ledger file #1524

Closed

simonmichael force-pushed the master branch 8 times, most recently from 56bc295 to 01f9c70 Compare July 11, 2021 09:26

simonmichael closed this May 19, 2025

simonmichael removed needs-impact-analysis To unblock: needs analysis of interactions with other features, users, ecosystem needs-testing To unblock: needs more developer testing or general usage needs-rebase To unblock: needs to be rebased against latest master branch labels May 19, 2025

simonmichael mentioned this pull request Jun 14, 2026

Support simple arithmetic expressions in posting amounts #2648

Open

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

ag-eitilt commented Dec 7, 2018

Uh oh!

ag-eitilt commented Dec 7, 2018

Uh oh!

simonmichael commented Dec 8, 2018

Uh oh!

Uh oh!

simonmichael Dec 8, 2018

Choose a reason for hiding this comment

Uh oh!

simonmichael Dec 8, 2018

Choose a reason for hiding this comment

Uh oh!

ag-eitilt Dec 8, 2018

Choose a reason for hiding this comment

Uh oh!

ag-eitilt Dec 8, 2018

Choose a reason for hiding this comment

Uh oh!

ag-eitilt commented Dec 14, 2018

Uh oh!

simonmichael commented Dec 15, 2018

Uh oh!

simonmichael commented Jan 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonmichael commented Feb 15, 2019

Uh oh!

simonmichael commented Feb 16, 2019

Uh oh!

lestephane commented Nov 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShrykeWindgrace commented Dec 16, 2019

Uh oh!

l29ah commented Nov 6, 2023

Uh oh!

simonmichael commented Nov 6, 2023

Uh oh!

simonmichael commented Nov 6, 2023

Uh oh!

simonmichael commented May 17, 2025

Uh oh!

l29ah commented May 17, 2025

Uh oh!

simonmichael commented May 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

simonmichael commented Jan 5, 2019 •

edited

Loading

lestephane commented Nov 7, 2019 •

edited

Loading