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

go_metalinter_deadline setting not respected #1142

Closed
horgh opened this issue Dec 9, 2016 · 3 comments
Closed

go_metalinter_deadline setting not respected #1142

horgh opened this issue Dec 9, 2016 · 3 comments

Comments

@horgh
Copy link
Contributor

horgh commented Dec 9, 2016

Behavior

I have changed my go_metalinter_deadline to 15s. However, I found that metalinter still times out after 5 seconds.

I believe this is because gometalinter defaults to a timeout of 5s, and in lint.vim there is a comment that says "we add deadline only for sync mode". I wonder if in the past gometalinter had no default deadline? It seems like we need it regardless.

I verified that if I move the line that adds --deadline to the command above if go#util#has_job() && has('lambda'), that gometalinter uses my 15s deadline and gometalinter can complete.

It seems like quite a small change to adjust this, but I thought I should make an issue rather than a PR in case there is a reason I do not understand that it is this way. If you like, I can make a PR.

Thank you for your time. This project is hugely useful!

Steps to reproduce:

  1. Have a slow computer :)
  2. Set g:go_metalinter_deadline = "15s" in vimrc
  3. Open a Go file
  4. Save it, see "vim-go: linting started..."
  5. Wait 5 seconds
  6. See WARNING: deadline exceeded [etc] (after 5 seconds)

I normally develop on a faster machine, but I've been doing a little on an older server as I am not currently at home.

Configuration

Add here your current configuration and additional information that might be
useful, such as:

  • vimrc you used to reproduce:
set nocompatible

execute pathogen#infect()

let g:go_fmt_command = "goimports"
let g:go_metalinter_autosave = 1
let g:go_metalinter_autosave_enabled = ['vet', 'golint', 'errcheck']
let g:go_metalinter_deadline = "15s"

set statusline+=%#goStatuslineColor#
set statusline+=%{go#statusline#Show()}
set statusline+=%*
  • vim version: 8.0.95
  • vim-go version: 1.10
  • go version: 1.7.3
@fatih
Copy link
Owner

fatih commented Dec 17, 2016

Hi @horgh

As you said it's a very small change indeed. The reason it's just for sync mode is that because async linting doesn't block anymore, so I thought it should not have any kind of deadline at all.

I'm ok if you open a PR. But please make it so the following logic applies

  • If async, the deadline should be passed only if the has defined g:go_metalinter_deadline itself
  • If sync, the deadline should be passed as the default 5s value, but if the user passed g:go_metalinter_deadline it should be overridden.

For async case you can use the get(g:, 'go_metalinter_deadline', 0) function, which returns the value of go_metalinter_deadline, but if it doesn't exist it returns the value 0. So you can check then the return value and only set --deadline if the return value is non null.

For sync case you can use get(g:, 'go_metalinter_deadline', "5s") and that's it. If the user applied anything it'll be the user value, otherwise it defaults to "5s".

Also you have to remove these lines from the file:

if !exists("g:go_metalinter_deadline")
  let g:go_metalinter_deadline = "5s"
endif

because otherwise the get(...) functions above would give us false positives. Lastly please do not forget to update our docs under doc/vim-go.txt and check if the documentation there is still valid.

@horgh
Copy link
Contributor Author

horgh commented Dec 18, 2016

Thanks very much for the background and for the pointers about how to do this!

I will submit a PR shortly.

fatih added a commit that referenced this issue Jan 7, 2017
#1142: go_metalinter_deadline applies in async mode now
@horgh
Copy link
Contributor Author

horgh commented Feb 20, 2017

This was merged. Thank you!

@horgh horgh closed this as completed Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants