-
Notifications
You must be signed in to change notification settings - Fork 54
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
Align currency with no decimal point correctly #160
Conversation
I see the problem (and thanks for the contribution), but I don't think this is the right solution. Yet. I don't typically use anything without a decimal separator so I hadn't noticed this before. I tried it is some of my ledgers and the "off by one" result is sometimes off by one, sometimes off by 2, and sometimes off by 3. So basically the whole thing is screwed. For example if you switch Rather than a 1 character stop gap I think we need to figure out why we're not calculating the width right at all. |
Agreed! I'll try and dig into the root cause of this when I find some time. |
I think this is due to variable unicode character byte length. See:
Note that in UTF-8 $ is one byte, £ is two, and € is three.
If we change:
To (stolen from
This appears to fix it:
|
Numbers with a space after them are still mis-aligned by 1, though:
|
This seems to be because So:
Gives us:
|
ed13e8e
to
e14299d
Compare
e14299d
to
0445eae
Compare
In my incredulity over not having noticed the pound sign being a multi-byte character it has come to my attention that in my own personal finance records (where I handle a lot of currencies but only a trivial about of travel related things in Pounds) I've been using Also an even further side note, apparently historically pound sterling (and ever other numbers after decimalization) were supposed to use In the mean time this sounds like a much better diagnosis and fix. I'll give it a spin when I get a minute, but thanks for digging in and finding the real issue. Did you poke around to see if we may be making the same mistake in any other calculations? |
Sounds like a minefield :-)
That's great! You're most welcome.
I couldn't see anything obvious, the other use of |
The docs say:
But this was not the behaviour I observed when calling
LedgerAlign
, there seemed to be an off by one error. For example, withg:ledger_align_at = 50
, I would get:This PR subtracts one from pos if there is no decimal point to give:
Which is what I expected.
I've made this PR with what corrects this for me, but I'm not familiar with the codebase so if I've overlooked something any feedback or advice is welcome.