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

Add support for numbers in account names #19

Merged
merged 4 commits into from
Jun 17, 2019
Merged

Add support for numbers in account names #19

merged 4 commits into from
Jun 17, 2019

Conversation

lockjs
Copy link
Contributor

@lockjs lockjs commented Jun 11, 2019

Previosuly given the account name Assets:Bank:Santander-123 auto complete would suggest Assets:Bank:Santander-

If the account name was Assets:Bank:Santander-123-Current-Account then auto complete would suggest Assets:Bank:Santander- and -Current-Account

The updated regexp prevents the account name being split so that Assets:Bank:Santander-123-Current-Account will be suggested

@Lencerf
Copy link
Owner

Lencerf commented Jun 16, 2019

Since Beancount actually allows any non-whitespace unicode characters in the account name, maybe [A-Za-z:]+\\S+|\"([^\\\\\"]|\\\\\")*\" is a better choice?

@lockjs
Copy link
Contributor Author

lockjs commented Jun 16, 2019

I Agree, seems like a better option and would resolve #20

I'll update the pull-request tomorrow.

Do you want me to bump the version number as well?

@Lencerf
Copy link
Owner

Lencerf commented Jun 16, 2019

@lockjs Thanks!
I will handle the version number and update README.

Root account names can only contain letters, starting with a Capital
Sub account names can contain any non-whitespace character
@lockjs
Copy link
Contributor Author

lockjs commented Jun 17, 2019

Updated the regex as you suggested.

I've also modified the language file to correctly highlight account names. You can see the non-ascii charachters weren't highlighted in the example in #20

@Lencerf Lencerf merged commit d732d24 into Lencerf:master Jun 17, 2019
This pull request was closed.
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