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

use window id instead of number in all async jobs #1734

Merged
merged 1 commit into from
Apr 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions autoload/go/job.vim
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
function go#job#Spawn(args)
let cbs = {}
let state = {
\ 'winnr': winnr(),
\ 'winid': win_getid(winnr()),
\ 'dir': getcwd(),
\ 'jobdir': fnameescape(expand("%:p:h")),
\ 'messages': [],
Expand Down Expand Up @@ -104,15 +104,20 @@ function go#job#Spawn(args)
let cbs.close_cb = function('s:close_cb', [], state)

function state.show_errors(job, exit_status, data)
let l:winid = win_getid(winnr())
call win_gotoid(self.winid)

let l:listtype = go#list#Type(self.for)
if a:exit_status == 0
call go#list#Clean(l:listtype)
call win_gotoid(l:winid)
Copy link
Owner

Choose a reason for hiding this comment

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

This is slightly confusing. We jump two times. First to the windows where the job was started, and then to the current window. Why do we have a logic like this? If we jump to the window the job was started, I wouldn't want to go back, but I think here we jump back because there are no errors right ? Maybe we should have an if .. else ... clause instead of jumping twice.

Copy link
Collaborator Author

@bhcleek bhcleek Mar 22, 2018

Choose a reason for hiding this comment

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

Assume the user has 2 windows open. While in window 1, the user runs an async command whose list type is locationlist. Before the async command completes, the user moves to window 2. Then the async command completes successfully. We need to clean the list of window 1 (hence the first jump). And we want to avoid stealing window focus, so we make window 2 active again before returning from the callback.

return
endif

let l:listtype = go#list#Type(self.for)
if len(a:data) == 0
call go#list#Clean(l:listtype)
call win_gotoid(l:winid)
return
endif

Expand All @@ -132,10 +137,11 @@ function go#job#Spawn(args)
if empty(errors)
" failed to parse errors, output the original content
call go#util#EchoError(self.messages + [self.dir])
call win_gotoid(l:winid)
return
endif

if self.winnr == winnr()
if self.winid == l:winid
call go#list#Window(l:listtype, len(errors))
if !self.bang
call go#list#JumpToFirst(l:listtype)
Expand Down
9 changes: 7 additions & 2 deletions autoload/go/jobcontrol.vim
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function! s:spawn(bang, desc, for, args) abort
let job = {
\ 'desc': a:desc,
\ 'bang': a:bang,
\ 'winnr': winnr(),
\ 'winid': win_getid(winnr()),
\ 'importpath': go#package#ImportPath(),
\ 'state': "RUNNING",
\ 'stderr' : [],
Expand Down Expand Up @@ -98,6 +98,9 @@ endfunction
" on_stderr handler. If there are no errors and a quickfix window is open,
" it'll be closed.
function! s:on_exit(job_id, exit_status, event) dict abort
let l:winid = win_getid(winnr())
call win_gotoid(self.winid)

let status = {
\ 'desc': 'last status',
\ 'type': self.status_type,
Expand Down Expand Up @@ -134,6 +137,7 @@ function! s:on_exit(job_id, exit_status, event) dict abort
endif

execute cd . fnameescape(dir)
call win_gotoid(l:winid)
return
endif

Expand All @@ -152,11 +156,12 @@ function! s:on_exit(job_id, exit_status, event) dict abort
if !len(errors)
" failed to parse errors, output the original content
call go#util#EchoError(std_combined[0])
call win_gotoid(l:winid)
return
endif

" if we are still in the same windows show the list
if self.winnr == winnr()
if self.winid == l:winid
call go#list#Window(l:listtype, len(errors))
if !empty(errors) && !self.bang
call go#list#JumpToFirst(l:listtype)
Expand Down
9 changes: 4 additions & 5 deletions autoload/go/lint.vim
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ function! s:lint_job(args, autosave)
\ 'exited': 0,
\ 'closed': 0,
\ 'exit_status': 0,
\ 'winnr': winnr(),
\ 'winid': win_getid(winnr()),
\ 'autosave': a:autosave
\ }

Expand Down Expand Up @@ -308,15 +308,14 @@ function! s:lint_job(args, autosave)
endif
endfunction


function state.show_errors()
let l:winnr = winnr()
let l:winid = win_getid(winnr())

" make sure the current window is the window from which gometalinter was
" run when the listtype is locationlist so that the location list for the
" correct window will be populated.
if self.listtype == 'locationlist'
exe self.winnr . "wincmd w"
call win_gotoid(self.winid)
endif

let l:errorformat = '%f:%l:%c:%t%*[^:]:\ %m,%f:%l::%t%*[^:]:\ %m'
Expand All @@ -331,7 +330,7 @@ function! s:lint_job(args, autosave)
" start of this function avoids the perception that the quickfix window
" steals focus when linting takes a while.
if self.autosave
exe l:winnr . "wincmd w"
call win_gotoid(self.winid)
endif

if get(g:, 'go_echo_command_info', 1)
Expand Down
31 changes: 20 additions & 11 deletions autoload/go/test.vim
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function! go#test#Test(bang, compile, ...) abort
let job_args = {
\ 'cmd': ['go'] + args,
\ 'bang': a:bang,
\ 'winnr': winnr(),
\ 'winid': win_getid(winnr()),
\ 'dir': getcwd(),
\ 'compile_test': a:compile,
\ 'jobdir': fnameescape(expand("%:p:h")),
Expand Down Expand Up @@ -242,11 +242,16 @@ endfunction
" a quickfix compatible list of errors. It's intended to be used only for go
" test output.
function! s:show_errors(args, exit_val, messages) abort
let l:listtype = go#list#Type("GoTest")
if a:exit_val == 0
call go#list#Clean(l:listtype)
return
endif
let l:winid = win_getid(winnr())

call win_gotoid(a:args.winid)

let l:listtype = go#list#Type("GoTest")
if a:exit_val == 0
call go#list#Clean(l:listtype)
call win_gotoid(l:winid)
return
endif

" TODO(bc): When messages is JSON, the JSON should be run through a
" filter to produce lines that are more easily described by errorformat.
Expand All @@ -266,14 +271,18 @@ function! s:show_errors(args, exit_val, messages) abort
" failed to parse errors, output the original content
call go#util#EchoError(a:messages)
call go#util#EchoError(a:args.dir)
call win_gotoid(l:winid)
return
endif

if a:args.winnr == winnr()
call go#list#Window(l:listtype, len(errors))
if !empty(errors) && !a:args.bang
call go#list#JumpToFirst(l:listtype)
endif
if a:args.winid != l:winid
call win_gotoid(l:winid)
return
endif

call go#list#Window(l:listtype, len(errors))
if !empty(errors) && !a:args.bang
call go#list#JumpToFirst(l:listtype)
endif
endfunction

Expand Down