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 --clean to avoid loading ftplugins #119

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Add --clean to avoid loading ftplugins #119

merged 1 commit into from
Feb 19, 2021

Conversation

nfischer
Copy link
Contributor

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

@google-cla google-cla bot added the cla: yes label Feb 17, 2021
@nfischer
Copy link
Contributor Author

@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
Copy link
Contributor

dbarnett commented Feb 18, 2021

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

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. This falls back
to trying without the '--clean' switch if vim fails to start up.

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
Copy link
Contributor Author

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.

# If vim exited this quickly, it probably means we passed a switch it
# doesn't recognize. Try again without the '--clean' switch since this is
# new in 8.0.1554+.
self.start_command.remove('--clean')
Copy link
Contributor

@dbarnett dbarnett Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I'm thinking of eventually plumbing through and posting a warning notice in the final output if we couldn't use --clean.

@dbarnett
Copy link
Contributor

That'll do the trick! Thanks!

Merging.

@dbarnett dbarnett merged commit 58e4340 into google:master Feb 19, 2021
@nfischer nfischer deleted the fix-dont-run-plugins branch February 20, 2021 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vroom is using settings in ftplugin/ directory
2 participants