Add additional metalinter options

[svanharmelen]

1. Add the option to run the metalinter on save
2. Add the option to limit the output to the currently active buffer

There is a outstanding PR with the gometalinter repo to add some needed logic for this: alecthomas/gometalinter#71 So I assume this PR should not be merged before that one is merged.

Also this the first vimscript code I’ve ever written, so I guess this could be done a little cleaner. Especially the implementation of the go#lint#GometaAutoSave func doesn’t feel all that clean to me, but I’m not sure how to do this in a different/better way. So if you have any pointers, I’ll be happy to take them 😉

[svanharmelen]

@fatih just tried to make things a little cleaner by using a different approach. Please let me know any feedback you might have. Thx!

P.S. I didn't squash on purpose so that both approaches are still visible, but will squash before (if) you merge this if you want...

[svanharmelen]

@fatih PR alecthomas/gometalinter#71 was just merged, so there are no external dependencies blokking this PR anymore...

[fatih]

Hi @svanharmelen

This looks good. I have some comments about it:

• I'm not sure I want the new variable: g:go_metalinter_autosave_enabled, as it pollutes the setting namespace for no reason and we already have g:go_metalinter_enabled. What was your reason you add it?
• We should remove the call go#list#JumpToFirst() statement when autosave is enabled, just add an if clause. Otherwise it's annoying when the cursor position changes when we hit save
• We should refactor the command creation into a new function (i.e: function s:gometa_cmd()) which would return a populated gometalinter command. Right now we are using the same set of commands in both functions (GoMetaAutoSave and GoMeta), and I don't like that we have identical copies there.

Thanks btw for your work, I think it's a really awesome contribution and I'm looking forward to use it :)

[svanharmelen]

Thanks for the review @fatih! Let me reply to your comments...

1. The reason for adding the g:go_metalinter_autosave_enabled variable is that I see different use cases for the different ways to use (call) the metalinter and without having two separate variables for g:go_metalinter_enabled and g:go_metalinter_autosave_enabled, I don't think both use cases could be fully supported (as they currently are in this PR). These are the use cases I'm thinking about:
   • During coding, I would like some direct feedback when saving a file. In those cases I think that the direct feedback should only report findings of the file being saved and (most likely) only uses a few lightweight and/or fast linters.
   • When I'm done coding and want to finish up a package/project, I would like to call :GoMetaLinter manually to get more in depth feedback about the whole package. So this time I would expect feedback from all files in the package and would like to use a different set of linters (a lot more and some of the heavier ones).
2. Yes, I think your right on that one... But simply using an if statement is maybe not enough as that changes the behaviour for both use cases (see reply on the first point as well).
3. First I thought about changing the signature of function! go#lint#Gometa(...) to something like function! go#lint#Gometa(autosave, ...) where autosave would be a bool that could be used to change behaviour according to the call that was made. But later I saw some difficulties with this approach if you want to support both use cases simultaneously. So that's why I changed it to having two separate functions. Will have another look following your pointer.

Let me know your thoughts about all this and if this makes sense to you as well.

Cheers!

[svanharmelen]

@fatih just updated the PR with another approach which removes the duplicate logic and handles everything within one function. Looks pretty clean to me, so let me know your thoughts...

[fatih]

Better: if !a:autosave

[svanharmelen]

That indeed reads (and looks) a little better...

[fatih]

@svanharmelen this is now in a really good shape :) I've made a comment, once that is done I think it's good to go.

[svanharmelen]

@fatih cool! Updated and squashed the commits...

[fatih]

@svanharmelen thanks for your awesome contribution. This is an awesome addition which I'm going to use myself a lot :)

[svanharmelen]

👍 😃