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.

Sign up for GitHub

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

Jump to bottom

Use absolute paths for gometalinter, so that filenames resolve correctly #565

Merged
merged 1 commit into from
Oct 12, 2015
Merged

Use absolute paths for gometalinter, so that filenames resolve correctly #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 12 additions & 16 deletions autoload/go/lint.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ if !exists("g:go_metalinter_enabled")
let g:go_metalinter_enabled = ['vet', 'golint', 'errcheck']
endif

if !exists("g:go_metalinter_path")
let g:go_metalinter_path = "./..."
endif

if !exists("g:go_golint_bin")
let g:go_golint_bin = "golint"
endif
Expand All @@ -18,31 +14,31 @@ if !exists("g:go_errcheck_bin")
let g:go_errcheck_bin = "errcheck"
endif

function! go#lint#Gometa(path_to_lint) abort
function! go#lint#Gometa(...) abort
if a:0 == 0
let goargs = expand('%:p:h')
Copy link
Owner

Choose a reason for hiding this comment

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

You only pass the current file's path. Previously we were recursively looking for all files under the current buffer's directory (with ./...). This means our :GoMetaLinter functionality has changed. I've added g:go_metalinter_path just for that, because for my case I want that it goes and lints all files.

I think I don't get an error because of this simple line in my vimrc:

autocmd BufEnter * silent! lcd %:p:h

So the fix here is to return absolute paths from gometalinter, not passing a folder just to make it return absolute paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of the other linters do recursive linting by default, why is GoMetaLinter a special case? There's no option to tell gometalinter to output full paths, so that's not a possible solution.

Passing a single file to gometalinter is not valid input, you must pass it a path (./... is a path). You should still be able to call GoMetaLinter ./... to get that behaviour, like some of the other linters.

I don't like the idea of forcing people to add stuff to their vimrc to get default behaviour working, and changing the working directory by default will produce unintended behaviour for users.

I understand not wishing to introduce breaking behaviour, but this is pretty fresh code, so there are probably not too many people affected, and at least this behaviour is consistent with the other linters.

PS: I fixed the errcheck output in gometalinter, so my comment above does not apply now.

Copy link
Owner

Choose a reason for hiding this comment

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

None of the other linters do recursive linting by default, why is GoMetaLinter a special case? There's no option to tell gometalinter to output full paths, so that's not a possible solution.

We know the current directory via Vim. So we can prepend it to gometalinter output if we want, but I think this would make it more complex than simpler.

I understand not wishing to introduce breaking behaviour, but this is pretty fresh code, so there are probably not too many people affected, and at least this behaviour is consistent with the other linters.

Thinking now I think this is fair. Just let me use it a little bit and get a feeling for this.

else
let goargs = go#util#Shelljoin(a:000)
endif

let meta_command = "gometalinter --disable-all"
if empty(g:go_metalinter_command)
let bin_path = go#path#CheckBinPath("gometalinter")
if empty(bin_path)
return
let bin_path = go#path#CheckBinPath("gometalinter")
if empty(bin_path)
return
endif

if empty(g:go_metalinter_enabled)
echohl Error | echomsg "vim-go: please enable linters with the setting g:go_metalinter_enabled" | echohl None
return
return
endif

for linter in g:go_metalinter_enabled
let meta_command .= " --enable=".linter
endfor


" by default we search for all underlying files
let path = g:go_metalinter_path
if !empty(a:path_to_lint)
let path = a:path_to_lint
endif

let meta_command .= " " . path
let meta_command .= " " . goargs
else
" the user wants something else, let us use it.
let meta_command = g:go_metalinter_command
Expand Down
18 changes: 4 additions & 14 deletions doc/vim-go.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,10 @@ COMMANDS *go-commands*
:GoMetaLinter [path]

Calls the underlying `gometalinter` tool and displays all warnings and
errors in a quickfix window. By default it lints the files for the path
`./...` . This can be changed with the 'path' argument temoporary or with the
|g:go_metalinter_path| variable permanently. By default the following
linters are enabled: "'vet', 'golint', 'errcheck'". This can be changed
with the |g:go_metalinter_enabled| variable. To override the command
completely use the variable |g:go_metalinter_command|
errors in a quickfix window. By default the following linters are enabled:
"'vet', 'golint', 'errcheck'". This can be changed with the
|g:go_metalinter_enabled| variable. To override the command completely use
the variable |g:go_metalinter_command|


===============================================================================
Expand Down Expand Up @@ -847,15 +845,7 @@ an advanced settings and is for users who want to have a complete control
over of how `gometalinter` should be executed. By default it's empty.
>
let g:go_metalinter_command = ""
<
*'g:go_metalinter_path'*

Defines the default path to be linted for the |GoMetaLinter| command. By
default it's set to `./...`, which recursively lints all files underd the
current directory.
>
let g:go_metalinter_path = "./..."
<

===============================================================================
TROUBLESHOOTING *go-troubleshooting*
Expand Down
2 changes: 1 addition & 1 deletion ftplugin/go/commands.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ command! -nargs=1 -bang -complete=customlist,go#package#Complete GoImport call g
command! -nargs=* -bang -complete=customlist,go#package#Complete GoImportAs call go#import#SwitchImport(1, <f-args>, '<bang>')

" -- linters
command! -nargs=* GoMetaLinter call go#lint#Gometa('')
command! -nargs=* GoMetaLinter call go#lint#Gometa(<f-args>)
command! -nargs=* GoLint call go#lint#Golint(<f-args>)
command! -nargs=* -bang GoVet call go#lint#Vet(<bang>0, <f-args>)
command! -nargs=* -complete=customlist,go#package#Complete GoErrCheck call go#lint#Errcheck(<f-args>)
Expand Down