-
Notifications
You must be signed in to change notification settings - Fork 55
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
Allow completion from declared accounts #59
Conversation
Mmmh, account names can contain spaces, so I cannot safely use split('[, \t]') … will fix this tomorrow. |
3b2db61
to
63aa71b
Compare
Should be fixed now by replacing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this is a very useful feature.
Can you re-indent your changes to match the rest of the file (inside while loop)?
When using ledger with the `--strict` option, you need to declare all your used accounts. These declarations can additionally be used to complete account names. ledger#declared_accounts() works a lot like ledger#transactions(), on which it is based. s:collect_completion_data() can then take the list of declared accounts as an initial value to which it then adds the accounts used in transactions. Signed-off-by: Roland Hieber <rohieb@rohieb.name>
63aa71b
to
5a41452
Compare
On 14.10.2017 18:31, Johann Klähn wrote:
Can you re-indent your changes to match the rest of the file (inside
while loop)?
Oh, I overlooked that.
I think you could also use the |{stopline}| argument to `search()̀ :
while lnum != 0
let lnum = search('^account\s', 'cW', end)
...
endw
Yes, I think it makes sense especially with large ledger files.
+function! ledger#declared_accounts(...)
+ if a:0 == 2
+ let lnum = a:1
+ let end = a:2
I would have thought that |end| is a reserved keyword in vim script. But
this does not seem to be the case. 😉
Mmh, vim does highlight it as a keyword when I use the variable in if
clauses... renamed to `lend` for less confusion.
Thank you for the feedback, I've updated the branch.
|
I'm also thinking if it would be a good idea to follow the include statements, and look for account completions in those files also. This way, when using |
Hey @rohieb thanks for taking the time to contribute this. I've recently stepped in to help maintenence of this plugin along a bit and am wondering what the status is. It looks like most of the PR feedback has been addressed. Is that your take too? I agree following includes (or better yet, some way to scan all project files because in my case I'm in the included file and want to complete from all accounts) but I don't think that should hold up merging this PR to add that feature (which might take a little ironing out). Is there any reason we can't go ahead with merging this as it stands? |
Thanks for coming back to this :-) I think it looks fine as-is. |
When using ledger with the
--strict
option, you need to declare all your used accounts. These declarations can additionally be used to complete account names.