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 a command to whitelist a 'runtimepath' entry that is required during tests #4

Open
roryokane opened this issue Oct 26, 2014 · 3 comments

Comments

@roryokane
Copy link
Contributor

Currently, Bisectly does not reliably support finding the cause of a bad interaction with another plugin – it only supports finding the cause of a bad change to base Vim.

For example, in this Stack Overflow question, the asker was using the ag.vim plugin. He used to use ag.vim’s :Ag command to open a window, and then :cnext to move to the next search result, but these days, :cnext after :Ag is doing something different. That asker stores his vimrc in a Git repo, so he should be able to use git bisect, but if he didn’t use a Git repo, then he might want to use Bisectly to see if a new plugin changed the behavior of ag.vim.

So he might run :Bisectly. But then, what if Bisectly disables the ag.vim plugin? First the user runs :Ag someSearch, intending to run :cnext next and see whether it exhibits the old or new behavior. But since ag.vim is disabled, the :Ag command will fail. What should he type? :Zombies or :Unicorns? Neither applies. If he types :Z, then Bisectly will eventually narrow the problem down to ag.vim, though it is not the actual cause of the problem. If he types :U, then Bisectly will only work if coincidentally, the problematic plugin always happens to be enabled whenever ag.vim is, until they are the last two plugins – which is extremely unlikely. So either way, Bisectly will give a bogus result.

One solution would be to add a third command for during Bisectly tests, :CantRunTestCase or :ImpossibleUniverse or something. This would tell Bisectly that it has disabled a prerequisite for the test. It might then do a binary search to find the prerequisite(s), followed by resuming its normal binary search after identifiying the 'runtimepath' entries to never disable.

But as an easier-to-implement solution that is still useful, Bisectly could implement a new command :BisectlyWithWhitelist that additionally takes a list of entries of the 'runtimepath' that should never be removed during the following tests. Then, the person in the example could look up the 'runtimepath' entry for ag.vim, and then do BisectlyWithWhitelist '~/.vim/bundle/ag.vim''. Since that would prevent ag.vim from being disabled during the tests,:Zombiesand:Unicorns` would always be meaningful, and the binary search would work.

I think a command that takes an argument, like :BisectlyWithWhitelist, would be better than an option, like g:bisectly_runtimepath_whitelist, since the user might forget that he had previously set the contents of that option, and that might mess up future tests.

For completeness, the whitelist should support comma-separated entries, just like 'runtimepath' does, so that multiple plugins can be whitelisted.

@roryokane roryokane changed the title Add a command to whitelist a runtimepath entry that is required during tests Add a command to whitelist a 'runtimepath' entry that is required during tests Oct 27, 2014
@dahu
Copy link
Owner

dahu commented Oct 27, 2014

The existing g:bisectly{'always_on'} option was intended for this purpose. Does it meet your needs?

@roryokane
Copy link
Contributor Author

Thanks, that looks like it would work. It just needs to be added to the documentation. Right now that option is mentioned neither in the README nor in the help documentation. If a user can’t discover a feature, it’s the same to them as if it doesn’t exist.

@dahu
Copy link
Owner

dahu commented Oct 27, 2014

Right. Agreed. I built it in initially with the idea of pinning plugins
on/off, but never got around to putting together an example in the doco.
I've done simple rests with it and from memory it worked as expected. Being
a largely untested and wholly undocumented feature though, expect rough
patches.

Thinking of it another way: It was a feature awaiting a champion. :-) Sally
forth, brave knight! :-p

-- bairui
On Oct 28, 2014 7:11 AM, "Rory O’Kane" notifications@github.com wrote:

Thanks, that looks like it would work. It just needs to be added to the
documentation. Right now that option is mentioned neither in the README
https://github.com/dahu/bisectly/blob/master/README.asciidoc nor in the help
documentation
https://github.com/dahu/bisectly/blob/master/doc/bisectly.txt. A
feature nobody knows about might as well not exist.


Reply to this email directly or view it on GitHub
#4 (comment).

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

No branches or pull requests

2 participants