google / vroom

Launch vim tests
Apache License 2.0
274 stars 27 forks source link

Add --clean to avoid loading ftplugins #119

Closed nfischer closed 3 years ago

nfischer commented 3 years ago

This uses vim's '--clean' switch to avoid loading ftplugins, since this can affect behavior during vroom tests. This switch seems to avoid the issue in my local testing, whereas other things ('-i NONE', '--noplugin', etc.) don't seem to have the same effect.

I can't get vroom running in '--neovim' mode (nvim 0.4.4), so this doesn't attempt to address the issue in the neovim code path. neovim appears to define '--clean' differently from vim, so it's possible neovim may require a different fix.

I verified this still respects vroom's '--vimrc' switch while having the desired behavior of avoiding plugins.

Fixes #105

nfischer commented 3 years ago

@dbarnett This seems to work correctly for vim 8.1.2269 (the particular test case I had issues with passes, as do all the other vroom test cases I'm aware of). Can you advise on how to check availability of the '--clean' switch? Is there an easy way to check the vim version?

dbarnett commented 3 years ago

Can you advise on how to check availability of the '--clean' switch? Is there an easy way to check the vim version?

I would suggest to always attempt to start with --clean but on errors fall back to running without --clean. That way in the common case there's no overhead.

Alternatives would be to always run vim --version first (guaranteed overhead, and could have gotchas trying to check that way anyway), or to accept new --clean and --noclean args in vroom that the caller has to configure correctly (awkward for users and still takes effort to implement).

nfischer commented 3 years ago

PTAL. It looks like the default wait time is 500ms which is more than enough for vim to recognize unknown switches. I tested this locally by changing --clean to --someunknownarg (to force this error case) and the fallback code worked as expected 10/10.

dbarnett commented 3 years ago

That'll do the trick! Thanks!

Merging.