targets.vim icon indicating copy to clipboard operation
targets.vim copied to clipboard

Parsing arguments. Example: ci2na works and c2ina works as well

Open blasco opened this issue 4 years ago • 5 comments

The parsing is done here:

let s:char1 = nr2char(getchar())
    " If char1 is number update v:count1 and get next char
    if s:char1 =~# '^\d\+$'
        let s:parsedCount = s:char1
        let s:char1 = nr2char(getchar())
    else
        let s:parsedCount = v:count1
    endif

I also updated targets#o to use a:count instead of v:count1

blasco avatar Sep 29 '19 20:09 blasco

Hey, thanks for the PR. It looks like currently it would only support single digit counts in that position, right? Can you change it so ci12na works, too?

And it would be great to have a test for the new usage and have it documented in the Readme and help file. Do you think you can do that?

wellle avatar Sep 29 '19 21:09 wellle

I've updated it so the user can input as many numbers as he wants and also so the position can be before or after 'nl'. Not sure how to add tests, maybe you can refer me to some quick guide or give me a short explanation. First time doing vimscript here.

blasco avatar Oct 19 '19 21:10 blasco

Very nice, thank you! A couple of comments:

  • Every character we get from getchar() should be added to chars. This is important so we can properly fall back to non target mappings if nothing is set up within targets for the current trigger.
  • I'd suggest using two different variables for the different places where you do the parsing. The background is that Vim multiplies all given counts, and we probably should do the same here. For example type 2d2w, it will delete four words. So if a user now types d2a3n4a, you would parse 3 and 4 and get 2 from v:count1. It should then delete 2*3*4=24 arguments. (In the future we might consider passing the different counts into the target factories, so we (or a plugin) could change the meaning so that da2n3a would delete 3 arguments after skipping the current and next one.)
  • As for testing, cd into the test subdirectory and run make. The Makefile opens Vim and sources test.vim which contains all the test logic. At the bottom of that file you see multiple test functions like s:testMotionForce() being called. Each of those opens one in file, like test10.in and then runs some vim script to position the cursor and invoke some targets functionality. At the end of each such test the file gets written to an out file like test10.out and then compared in the Makefile against test11.ok. So in this case we might want to add a new test function like s:testCounts() which reads cases from test12.in and then writes output to test12.out etc.

wellle avatar Oct 20 '19 18:10 wellle

@blasco: Please let me know if you'd like me to take over some or all of those points :v:

wellle avatar Nov 07 '19 20:11 wellle

@wellle I'm unfortunately quite busy lately, could you please take over those tasks? I don't think I'll be available in the upcoming month, but I'll try after.

blasco avatar Nov 08 '19 06:11 blasco