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

Fix LedgerAlign corruptions #73

Merged
merged 2 commits into from
Jul 1, 2019
Merged

Fix LedgerAlign corruptions #73

merged 2 commits into from
Jul 1, 2019

Conversation

mildred
Copy link

@mildred mildred commented Mar 14, 2019

ledgerAlign could cause corruption in some lines where the account name was too long.

Mildred Ki'Lya added 2 commits March 14, 2019 22:51
This commit detect the location of the decimal point consistently in the
following circumstances (given g:ledger_decimal_sep='.'):

    -30 = 0
    456.78

It uses the decimal point location if it can be found, and if it cannot,
it uses the first character after the first group of digits.
When account names are too long, LedgerAlign was currupting the aligned
lines. This commit ensures that:

- In any case, if the account name is longer than the aligning column,
  the line is not corrupted and the amount is put at the end of the line
  (instead of putting it in the middle of the account name)

- In any case, it ensures that there is a minimum of 2 spaces between
  the account name and the amount.
@alerque
Copy link
Member

alerque commented Jun 12, 2019

Just ran into this myself! Amounts ended up interspersed inside account strings, oops!

I'd love to see this cleaned up and merged. See #75 for my proposal to help maintain this plugin.

@alerque
Copy link
Member

alerque commented Jul 1, 2019

Thanks for the contribution @mildred. I'm sorry it took so long to address.

I've been using these commits in my own use now for several weeks. As far as I'm concerned the plugin is fatally broken without it and works okay with it, so I'm going to go ahead and push this merge through right away. I'll probably refactor the code a little bit down the road, but I don't see any reason to hold up what seems to be a working improvement. Maybe people with short account trees or long line defaults might work around this bug, but clobbering the data format is a pretty serious offense.

@alerque alerque merged commit 56a3038 into ledger:master Jul 1, 2019
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 this pull request may close these issues.

2 participants