ledger / vim-ledger

Vim plugin for Ledger
GNU General Public License v2.0
372 stars 55 forks source link

support to change individual posting's transaction state #112

Closed kcwu closed 3 years ago

kcwu commented 4 years ago

I'm newbie to vimscript, so I may make stupid mistakes. Feel free to give me suggestions to improve the code.

Closes #81

alerque commented 4 years ago

Thanks for taking the time to contribute this @kcwu. Sorry I haven't had a chance to review it and put it though some paces yet. If anybody else does give this a test drive before I get around to it please be sure to comment.

kcwu commented 3 years ago

given

2020-03-06 Buy
  Expenses  $10.00
  Cash

and :call ledger#transaction_post_state_set(line('.'), '*') at the second line, it became

2020-03-06 Buy
  * Expenses  $10.00
  Cash

and :call ledger#transaction_post_state_set(line('.'), '') at the same line, it changed back to

2020-03-06 Buy
  Expenses  $10.00
  Cash

if repeatedly execute :call ledger#transaction_post_state_toggle(line('.'), '!* '), the second line will rotate among

And finally, if call transaction_post_state_set/transaction_post_state_toggle at the first line, they will fallback to transaction_state_set/transaction_state_toggle nicely.

alerque commented 3 years ago

Thanks, that's great. Made writing a set of test cases for this a snap!

Just one question about this one:

and :call ledger#transaction_post_state_set(line('.'), '') at the same line, it changed back to

2020-03-06 Buy
  Expenses  $10.00
  Cash

Was this supposed to be an empty string? Or a space?

:call ledger#transaction_post_state_set(line('.'), '')
:call ledger#transaction_post_state_set(line('.'), ' ')

In testing I found both forms work for clearing an existing state, but the former (empty string) manages to unformat the line as being a posting (deletes all indentation) if run on a posting with no state. The second form works in that scenario to leave it alone.

On the other hand the form with the space breaks if used on a transaction instead of a posting, inserting an extra unwanted space.

I don't feel too strongly about what form to use, but whatever form we go with should be safe to use to clear both postings and transactions.

alerque commented 3 years ago

Note the tests are now failing because two of them were written specifiably to catch the what seems like misbehavior to me. If you have this plugin and Vader installed in your personal RC file you can run tests locally like this:

# For NeoVIM
$ nvim --headless '+Vader! spec/*'

# For VIM
$ vim -N '+Vader! spec/*'

Or if you want more isolated testing you can run the RC file shenanigans from the workflow file.

kcwu commented 3 years ago

Because transaction_state_set's comment says removing the state altogether if a:char is empty, so I want to keep them support both.

The two cases are bugs -- one is mine and one is existing. Now I fixed them. Please take a look.

kcwu commented 3 years ago

(also rebased against master)