Closed kljohann closed 11 years ago
Wow, that’s quite a patch. There’s no way I can review it sensibly. :-/
I would really love to merge it but I’m too scared it might break things since we have nothing like a test suite. Are you sure your version still works in older vims too? How did you make sure the behavior hasn’t changed?
Re: the “Contributor” header, I’ll add a file for that and remove myself as well from the main file leaving only the original authors inside.
I'm using this version on my python files and tried to check most cases by hand but apart from that I have no way to check that the behaviour is the same. Regarding older vim versions I checked the patch notes of prior releases (7.3, 7.2, 7.1) and so far I did not find the patch where the stopline argument was introduced so I guess that is safe to use. The only thing I found is patch 7.3.608 ("winrestview() does not always restore the view correctly") but as indentexpr
will reset the cursor the call to winsaveview
may not be needed after all.
Of the few vim test frameworks available I like the approach of maxbrunsfeld/vimbot best for this kind of plugin (see maxbrunsfeld/vim-yankstack for an example of its use). I could write some tests in a separate pull request and we could run them against both implementations.
That sounds great! And actually something we should have anyway if we want to ever move forward.
Just a quick status update: I ran into some problems with vimbot and did not find the time to work on this. I still intend to do this eventually, but I cannot say when it will be ready.
So I think this is completely out of date now so I’m closing it. Feel free to rebase the refactorings – but thanks a lot for making this project tested. :)
No problem, I will resubmit this after #19 is decided on.
I did a code cleanup to increase readability and extensibility and fixed some issues on the way. For example support for patch 7.3.629 (
sw
can be set to-1
to followts
) and using thestopline
argument ofsearchpairpos
. As this is a near rewrite (although I tested this on my python files) please check that I did not break your use case. If everything works for you I will try to implement #5.