TAB completion not working for some accounts on ledger/test/input/drewr3.dat

[ednolan]

I tried to open the ledger test file ledger/test/input/drewr3.dat and try out the ledger-mode autocomplete. However, all the accounts starting with "Income:" failed to autocomplete. Additionally, when I evaluated (ledger-accounts-list-in-buffer), those accounts failed to appear in the list of accounts. The following input can minimally reproduce the problem:

; -*- ledger -*-

2011/01/05 Employer
  Assets:Checking                   $ 2000.00
  Income:Salary

Evaulating (ledger-accounts-list-in-buffer) on this text returns ("Assets:Checking")

[purcell]

I can confirm this regression, and I supect it was introduced in or around 37a9c83 by @DamienCassou - Damien, perhaps you could take a quick look please?

[DamienCassou]

TLDR

Workaround: either:

1. declare your account with the account directive, or
2. add an amount at the end of the posting.

Long explanation

Ledger's grammar requires reading more than a line to understand what the current line means. For example:

    foo bar

could be

• a posting in a transaction for an account named "foo bar" and an automatically-calculated amount (just like in @ednolan's example) or
• a directive foo with argument bar in a declaration (e.g., of a payee).

Ledger-mode works by searching for regular expressions matching each line, line by line. This is an approach that works in most cases but not with some cases because of the above.

To avoid having Ledger detect each directive as an account name which makes account completion very hard to use, my former PR stopped considering postings with no amount as new account names. The rationale was that it's much easier to either declare your account with the account directive or to add the amount in at least one posting for that account than to use account completion where many of the "account" names are not account names but directives or transaction comments.

[purcell]

I wasn't aware that this was the effect of the previous PR. That's a shame, because neither of the workarounds should be necessary. Directives are less common than postings without amounts, and @ednolan's case here is one that almost every ledger user might hit.

I feel like what we need is to eliminate the regexes that are used widely and inconsistently from elsewhere in ledger-mode and instead implement a proper parser for transactions and directives -- we could then use that throughout the code to do things like safely find all the account names mentioned in transactions.

[enderw88]

Sounds like an emacs native ledger.

On Sat, Nov 3, 2018 at 17:38 Steve Purcell ***@***.***> wrote: I wasn't aware that this was the effect of the previous PR. That's a shame, because neither of the workarounds should be necessary. Directives are less common than postings without amounts, and @ednolan <https://github.com/ednolan>'s case here is one that almost every ledger user might hit. I feel like what we need is to eliminate the regexes that are used widely and inconsistently from elsewhere in ledger-mode and instead implement a proper parser for transactions and directives -- we could then use that throughout the code to do things like safely find all the account names mentioned in transactions. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLIrz-syesUDRQEJlSk3I8M-PSefMks5urjcYgaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[purcell]

Sounds like an emacs native ledger.

Only as far as parsing is concerned. Imagine that when you parse the current transaction you get something like:

'((date . "2011/01/05")
  (effective-date . nil) 
  (payee . "Employer")
  (postings . (((account . "Assets:Checking") (commodity . "$") (amount . "2000.00"))
               ((account . "Income:Salary") (commodity . nil) (amount . nil))))

So now you can use let-alist or assoc etc. to extract data about the file and current transaction context.

[DamienCassou]

I wasn't aware that this was the effect of the previous PR.

I'm sorry, I should I have been explicit.

I feel like what we need is to eliminate the regexes [...] in ledger-mode and instead implement a proper parser

I agree. Do you think about implementing the parser in Emacs Lisp or reuse the one in C++ and the ledger lisp command?

[purcell]

I guess either. The latter might exclude hledger users, which would be bad. I'm not familiar enough with hledger to know.

[enderw88]

When I wrote all these regexs several years ago, I looked at doing a parser. I couldn’t generate a Backus Naur grammar to feed to a parser generator and I thought parsing entire files in Elisp would be very slow. This could be my lack of facility with BN notation. Calling out to ledger to generate lisp is problematic since during editing the file won’t parse most of the time. Since this seems to be mostly about account tab completion I suggest periodically updating the completion list using “ledger accounts”. Again you have to be very lenient about ledger failing whiling editing an xact. Ledger is fundamentally not designed for interactive use with a changing input.

On Sun, Nov 4, 2018 at 14:03 Steve Purcell ***@***.***> wrote: I guess either. The latter might exclude hledger users, which would be bad. I'm not familiar enough with hledger to know. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLNidTAEulM0WL9NJdqKx8lQiTqCyks5ur1Y3gaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[ngleb]

1. declare your account with the `account` directive, or

Unfortunately this doesn't work if accounts are declared in a separate file which is imported in the main file.

[enderw88]

This is a known issue. Not a bug, because I never tried to implement multiple file parsing.

On Thu, Dec 13, 2018 at 13:23 Gleb Nemshilov ***@***.***> wrote: 1. declare your account with the `account` directive, or Unfortunately this doesn't work if accounts are declared in a separate file which is imported in the main file. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLBVjZhNJKSHeMPc8txJaGUye_IqDks5u4qlEgaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[OskarSigvardsson]

I happened across this as well and I must say that this is a very serious regression. It makes it significantly more tedious to enter transactions, and it's a stated benefit of Ledger that you don't actually have to pre-declare accounts in order for it to work, it's flexible enough so you can add accounts with no fuss.

It may just be my personal case, but I would say it's a far more serious issue that tab completion wont complete the majority of my accounts than the fact that it might add directives to the completion list.

[enderw88]

Please give details of your setup. I use tab completion all the time with no issues.

On Fri, Mar 1, 2019 at 08:53 OskarSigvardsson ***@***.***> wrote: I happened across this as well and I must say that this is a very serious regression. It makes it significantly more tedious to enter transactions, and it's a stated benefit of Ledger that you don't actually have to pre-declare accounts in order for it to work, it's flexible enough so you can add accounts with no fuss. It may just be my personal case, but I would say it's a far more serious issue that tab completion wont complete the majority of my accounts than the fact that it might add directives to the completion list. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLPIiA8GS1VpzoaGkNM_5O-ETfqpqks5vSUz6gaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[OskarSigvardsson]

I'm having the exact same problem that everyone else in this issue is having, it has nothing to do with my setup. I tried it with emacs -Q and it happens there as well. Here's a minimal example to reproduce:

2019/03/01 Grocery store
    Assets:Checking       -$40
    Expenses:Groceries 

2019/03/02 Grocery store
    Assets:Checking       -$50
    Expenses:

If you have the point on the last line after Expenses: and press tab, it will not auto-complete with Groceries, because it does not understand that Expenses:Groceries is an account (which you can confirm by running ledger-accounts-list-in-buffer). However, if you format the first transaction like this

2019/03/01 Grocery store
    Assets:Checking       -$40
    Expenses:Groceries     $40

Then it works, now it recognizes Expenses:Groceries as an account. The issue is that most of my transactions are of that first format with the empty line. Of course I can solve it by manually adding account Expenses:Groceries line, but this did not use to be the case (which is why this is a regression) and it's clearly not how it should work.

[enderw88]

I will look into it but I do this every day with null amounts on postings. What version of EMACS and platform are you using ?

On Fri, Mar 1, 2019 at 09:47 OskarSigvardsson ***@***.***> wrote: I'm having the exact same problem that everyone else in this issue is having, it has nothing to do with my setup. I tried it with emacs -Q and it happens there as well. Here's a minimal example to reproduce: 2019/03/01 Grocery store Assets:Checking -$40 Expenses:Groceries 2019/03/02 Grocery store Assets:Checking -$50 Expenses: If you have the point on the last line after Expenses: and press tab, it will not auto-complete with Groceries, because it does not understand that Expenses:Groceries is an account (which you can confirm by running ledger-accounts-list-in-buffer). However, if you format the first transaction like this 2019/03/01 Grocery store Assets:Checking -$40 Expenses:Groceries $40 Then it works, now it recognizes Expenses:Groceries as an account. The issue is that most of my transactions are of that first format with the empty line. Of course I can solve it by manually adding account Expenses:Groceries line, but this did not use to be the case (which is why this is a regression) and it's clearly not how it should work. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLDr6Qj2cRwcZgoFO5b7ey2zCl45lks5vSVmwgaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[OskarSigvardsson]

@enderw88 I'm on Emacs 26.1 on Linux. But I'm clearly not the only one having this issue, there's other people reporting the same thing in this thread and @DamienCassou provided the explanation.

[ngleb]

I can confirm that I have the same issue. Emacs 27.0.50 (from master), ledger-mode from master, ledger 3.1.2.

I also think it was confirmed that this issue does exist above.

[enderw88]

I apologize. I had forgotten the details of this thread. I am very selective about accepting updates to the regexes for exactly this reason. I am not saying they are perfect, they’re not. But I haven’t seen anyone make an improvement that didn’t break something else.

[OskarSigvardsson]

Perfectly understandable. I realize it's a hard problem to solve.

Since Ledger (the i.e. the CLI) obviously need ot be able to tell the difference between directives and accounts, isn't a possible solution to have Ledger generate the list of accounts in a file (I don't know if the Ledger CLI has that built-in as functionality, but I would assume it would not be hard to implement even if it didn't) and use that for magic-tab-completion? If ledger is not available, you could fall back on the regexes. Would that be an acceptable solution?

[ngleb]

ledger accounts prints all account names.

[enderw88]

That was one of my suggestions above. The problem happens that ledger won’t parse if there are imbalances. So when you are in the middle of a transaction “ledger accounts “ won’t work. Maybe there is a middle ground using ledger to update the accounts list and ignoring failures when the file can’t be parsed. There would be corner cases where I can see it wouldn’t work. The would also increase dependence on ledger itself which isn’t used at all outside of reporting.

On Fri, Mar 1, 2019 at 10:10 OskarSigvardsson ***@***.***> wrote: Perfectly understandable. I realize it's a hard problem to solve. Since Ledger (the i.e. the CLI) obviously need ot be able to tell the difference between directives and accounts, isn't a possible solution to have Ledger generate the list of accounts in a file (I don't know if the Ledger CLI has that built-in as functionality, but I would assume it would not be hard to implement even if it didn't) and use that for magic-tab-completion? If ledger is not available, you could fall back on the regexes. Would that be an acceptable solution? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLCv5iZIm2NNfbfkiDufgFr--inc0ks5vSV79gaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[DamienCassou]

@OskarSigvardsson wrote:

It may just be my personal case, but I would say it's a far more serious issue that tab completion wont complete the majority of my accounts than the fact that it might add directives to the completion list.

I guess we are different: I want all my accounts and posting amounts to be declared (e.g., to avoid typos). As I don't create accounts anymore, the cost of declaring them is close to 0 now. Moreover, because I use directives and comments a lot, completion was completely broken for me. Apparently, @jwiegley agrees with you that I'm part of a minority of users. Unless you add new accounts constantly, I suggest you either declare your accounts or write at least one posting amount per account. I'm sorry I didn't see that it would be such a nuisance to several users.

@enderw88 wrote:

I am very selective about accepting updates to the regexes for exactly this reason. I am not saying they are perfect, they’re not. But I haven’t seen anyone make an improvement that didn’t break something else.

I agree with you. Whatever line-based regexp you choose, they will be problematic to either me or other people as I explained in #141 (comment). There is just no way that a line-based regexp can work in all cases because of Ledger's grammar.

A way to let developers improve the regexps (and the rest) is to invest in many unit tests (ledger-mode already has plenty and I added one in the offending PR).

@OskarSigvardsson

Since Ledger (the i.e. the CLI) obviously need ot be able to tell the difference between directives and accounts, isn't a possible solution to have Ledger generate the list of accounts.

This was proposed by @enderw88 already: #141 (comment)

[enderw88]

I went and did some quick tests on using "ledger accounts" to generate the account list for use in TAB completion.

I keep a huge ledger file around to troubleshoot the odd complaint. I generate it using gpp and some inculde directives in a script. It is every xact I have had since 2011, here are its stats:

09:21:58 ~/FinanceData > ledger -f entire.ledger stats
Time period: 2010-12-31 to 2019-04-15 (3027 days)

Files these postings came from:
/Users/cpearls/Documents/Finances/Ledger/entire.ledger

Unique payees: 2423
Unique accounts: 157

Number of postings: 47884 (16 per day)
Uncleared postings: 22

Days since last post: -43
Posts in last 7 days: 190
Posts in last 30 days: 602
Posts seen this month: 83

When I use that file to generate the accounts list I get the following:

09:21:36 ~/FinanceData > time ledger -f entire.ledger accounts > accounts

real 0m7.249s
user 0m6.693s
sys 0m0.551s

I ran it multiple time,and this time is typical. For comparison a "bal Expenses" command on the same file takes:

09:21:53 ~/FinanceData > time ledger -f entire.ledger bal Expenses > accounts

real 0m0.456s
user 0m0.417s
sys 0m0.024s

When I run the accounts command on just my calendar year 2018 file I get the following:

09:26:32 ~/FinanceData > time ledger -f CY2018.ledger accounts > accounts

real 0m0.327s
user 0m0.296s
sys 0m0.028s

I have gotten serious and emphatic complaints abut tab completion being slow, and taking less time than 0.327 seconds. 7 seconds it right out.

I don't see using ledger as the back end for the TAB completion as acceptable. An elisp parser would be much slower than ledger, certainly.

I would prefer to revert the previous change to the REGEXes that caused this problem as more people are complaining its side effects than complained about the problem the most recent change "fixed".

[DamienCassou]

I have gotten serious and emphatic complaints abut tab completion being slow, and taking less time than 0.327 seconds. 7 seconds it right out.

that's for sure :-). What about completing accounts from a cache and refreshing this cache from regular non-blocking calls to ledger?

I would prefer to revert the previous change to the REGEXes that caused this problem as more people are complaining its side effects than complained about the problem the most recent change "fixed".

I understand your point of view and won't fight it. But please take into account that it's very easy to workaround the problems introduced by my patch (either declare accounts or add a few posting amounts) whereas there is just no way to workaround the problems that my patch solved (beyond deleting all comments and directives). If my patch is reverted, my account completion candidates will grow from 168 accounts to 724 (with "accounts" such as "note 🗓 333.00€/m", "default" and "alias 18479482734") making completion unusable.

[ngleb]

I understand your point of view and won't fight it. But please take into account that it's very easy to workaround the problems introduced by my patch (either declare accounts or add a few posting amounts) whereas there is just no way to workaround the problems that my patch solved (beyond deleting all comments and directives). If my patch is reverted, my account completion candidates will grow from 168 accounts to 724 (with "accounts" such as "note spiral_calendar 333.00€/m", "default" and "alias 18479482734") making completion unusable.

I agree that leaving it as is would be a better choice. At least we have a workaround that doesn't involve changing too much. My only issue is that I have to adjust setup on several machines manually, so that's why I still had this problem.

[OskarSigvardsson]

@DamienCassou I totally understand that tab completion gets annoying if it includes all directives, but I want to point out that the solution of predeclaring accounts with the account directive is entirely non-obvious. I would never had thought of it if you hadn't mentioned it in this thread. Every Ledger tutorial and guide you read on the entire internet (as well as the manual) makes a point of you NOT having to do this: there's no limits to the amount of account you have, and you don't need to pre-declare them, you can just start using it in a transaction and everything works.

I'll share my experience: I updated ledger, and noticed the next time that I tried adding a transaction that tab-completion didn't work for the account i wanted to add. The thing that i had been using for ever had suddenly broken. I thought maybe it was because the account had a hyphen in it, so I search/replaced the account name with the hyphen to one without a hyphen. Didn't work. Next, I thought maybe it was because it was because the account was a third-level subaccount, so I search-replaced that instead. Also didn't work.

At that point, I figured that it must be that ledger either had a serious bug or that I had a strange configuration. I C-h c:ed to find out what function was running when i pressed tab. I then fired up the emacs debugger (actually, the first thing I did was read a tutorial on the emacs elisp debugger, as I've barely used it before) and eventually tracked the problem down to the regexes. I then came here ready to report the issue, before seeing that, of course, somebody had already reported it before me (in hindsight, I should probably have come to github earlier, but c'est la vie).

All in all, i spent something like 2 hours trying to figure out why tab completion in ledger-mode was broken. At no point did it occur to me to fix it using the account directive, until I saw your advice in this thread. And I'm still sort-of annoyed that I need to, I had never done this before and I still wouldn't do it if weren't for this bug (I love the flexibility of not having to). If I were less technically inclined or less eager to use Ledger for my personal finances, I simply would have concluded that ledger-mode is broken.

You mention that there is no simple workaround for your issue, but how about this: until we can get some sort of more advanced parsing (or running ledger accounts asynchronously), we make this a customization option. By default, it uses the old regex that included all accounts and directives, and we offer the option of using customize to change the regex to the more restrictive one that excludes directives? That way, you could make a one time customization to fix your issue, while the tab completion keeps working like it used to for the rest of the user base. We also add a mention of this to the ledger-mode info pages.

It's not an ideal solution, but until we have a better parsing solution, I think this is the best of a number of bad choices. What do you guys think about that solution?

[DamienCassou]

OskarSigvardsson <notifications@github.com> writes:

You mention that there is no simple workaround for your issue, but how about this: until we can get some sort of more advanced parsing (or running `ledger accounts` asynchronously), we make this a customization option. By default, it uses the old regex that included all accounts and directives, and we offer the option of using `customize` to change the regex to the more restrictive one that excludes directives?

For the sake of keeping code simple, I would vote against that and in favor of just reverting my patch.

…

-- Damien Cassou http://damiencassou.seasidehosting.st "Success is the ability to go from one failure to another without losing enthusiasm." --Winston Churchill

[ednolan]

When I use that file to generate the accounts list I get the following:

09:21:36 ~/FinanceData > time ledger -f entire.ledger accounts > accounts

real 0m7.249s
user 0m6.693s
sys 0m0.551s

I feel like maybe the reason this is slow is that ledger -f is doing more work than this use case needs. What if ledger was used as the backend for tab completion, but it had an optimized command line option that would only output a list of accounts? Admittedly that's outside the scope of just the emacs ledger-mode project itself.

[enderw88]

-f tells it what file to read. “accounts” is the command to just generate a list of accounts.

On Mon, Mar 4, 2019 at 09:20 Edward Nolan ***@***.***> wrote: When I use that file to generate the accounts list I get the following: 09:21:36 ~/FinanceData > time ledger -f entire.ledger accounts > accounts real 0m7.249s user 0m6.693s sys 0m0.551s I feel like maybe the reason this is slow is that ledger -f is doing more work than this use case needs. What if ledger was used as the backend for tab completion, but it had an optimized command line option that would only output a list of accounts? Admittedly that's outside the scope of just the emacs ledger-mode project itself. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLOGBFTQ9s8yfxKrLY92yOkhajFqaks5vTUfNgaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[ednolan]

Oh okay. Why is it so slow then? Even though the file is big, I feel like just parsing it for accounts shouldn't take 7 seconds, since it doesn't need to balance them.

[enderw88]

Good question. I haven’t dug into it.

On Mon, Mar 4, 2019 at 09:37 Edward Nolan ***@***.***> wrote: Oh okay. Why is it so slow then? Even though the file is big, I feel like just parsing it for accounts shouldn't take 7 seconds, since it doesn't need to balance them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA1rLHbVBB59FAzzHVLF5_yflOGZxwAtks5vTUvggaJpZM4YLC4t> .

-- Craig, Corona De Tucson, AZ [image: missile_flyout] enderw88.wordpress.com

[peti]

I have also wondered about the change in behavior and only by chance did I discover this thread. It's pretty annoying that such a breaking change just creeps in.

Anyhow, I don't like the new behavior that much, to be honest. If it were possible to keep the account declarations in a separate file, then I wouldn't mind declaring them -- especially since such a list can be trivially generated with ledger --, but since that does not seem to be possible, I'd much rather go back to the old completer.

[purcell]

ledger accounts prints all account names.

Unfortunately it doesn't print out accounts declared with account directives.

Re. the speed issue you're seeing, @enderw88, is it any faster if you pass --permissive in order to disable balance assertions?

Apart from those two issues, it might be feasible to use the ledger accounts command even if the current file were not completely well-formed, based on the assumption that new transactions are typically added at the end: I expect that in almost all cases one could pass the file up to the current transaction to ledger and get a reasonable result.

Overall, though, I agree that the best solution here would be to use a regexp that matches more liberally, so I've done that in #183 and will merge if the tests pass.

[purcell]

(@peti) If it were possible to keep the account declarations in a separate file, then I wouldn't mind declaring them -- especially since such a list can be trivially generated with ledger --, but since that does not seem to be possible, I'd much rather go back to the old completer.

This is actually possible, btw: you can have an accounts-list.ledger, full of account directives, and include it in your main .ledger file. Then, you can set ledger-accounts-file to tell ledger mode to find the accounts there.

[purcell]

Okay, this should be fixed in the latest code. There's a new test for this specific case.