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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

blasco
Copy link

@blasco blasco commented Sep 29, 2019

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

@wellle
Copy link
Owner

wellle commented Sep 29, 2019

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?

@blasco
Copy link
Author

blasco commented Oct 19, 2019

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.

@wellle
Copy link
Owner

wellle commented Oct 20, 2019

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
Copy link
Owner

wellle commented Nov 7, 2019

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

@blasco
Copy link
Author

blasco commented Nov 8, 2019

@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.

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