Lint vimscript, fix errors and warnings, add CI job to review PRs #1071
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Linting is not as good as full on integration testing, but this is a start. The vint linter for VimL is actually pretty good. It's noisy the first time you run it because it's picking about idiosyncrasies of VIM's language and wants all code to avoid even potential breakage, but it is all good practice and once everything is fixed it's pretty easy to use. Once installed it can be run locally using
vint .
from the project root.The CI job included here will run the linter in Github Actions and a bot will leave comments on PRs on the line of code where any lint issues are found.
There are a couple flat out bugs that it caught, but most of the changes here were just fixing warnings. I highly recommend this PR not be squashed! This touches a TON of code, but each commit is split out by the kind of lint issue being fixed and it will be much easier to understand the
git blame
output and potentiallygit bisect
if a problem was introduced if they are not lumped together.Please do make sure the plugin still runs as expected. I don't usually use it myself so I'm not a good one to put it through even quick paces.
If you poor through the diff it's probably a good idea to do so a commit at a time so it's not just a mess of changes ;-)
After it is merged I would recommend setting
master
up as a protected branch and requiring the CI job to pass on all PRs before merger. I can set that up in the Github settings is you approve (but I wont' mess with it unless you give the word).