From a2657879037d48bf35404f43b203fc9ad6eb8e5d Mon Sep 17 00:00:00 2001 From: Billie Cleek Date: Sun, 18 Mar 2018 22:20:10 -0700 Subject: [PATCH] Fix various race conditions when using terminal windows * Explicitly close the terminal window instead of assuming the terminal window is the current window and closing that. * Eliminate the outstanding job list in favor of using a more object oriented like approach. * Make sure the correct location list gets used instead of assuming that the current window is the window whose location list should be used. Fixes #1720 --- autoload/go/job.vim | 8 ++-- autoload/go/term.vim | 95 +++++++++++++++++++------------------------- 2 files changed, 45 insertions(+), 58 deletions(-) diff --git a/autoload/go/job.vim b/autoload/go/job.vim index 886f8bc45c..1fc5df8eb0 100644 --- a/autoload/go/job.vim +++ b/autoload/go/job.vim @@ -64,7 +64,7 @@ function go#job#Spawn(args) function! s:callback(chan, msg) dict call add(self.messages, a:msg) endfunction - " explicitly bind callback so that to state so that within it, self will + " explicitly bind callback to state so that within it, self will " always refer to state. See :help Partial for more information. let cbs.callback = function('s:callback', [], state) @@ -85,8 +85,8 @@ function go#job#Spawn(args) call self.show_errors(a:job, self.exit_status, self.messages) endif endfunction - " explicitly bind exit_cb so that to state so that within it, self will - " always refer to state. See :help Partial for more information. + " explicitly bind exit_cb to state so that within it, self will always refer + " to state. See :help Partial for more information. let cbs.exit_cb = function('s:exit_cb', [], state) function! s:close_cb(ch) dict @@ -98,7 +98,7 @@ function go#job#Spawn(args) call self.show_errors(job, self.exit_status, self.messages) endif endfunction - " explicitly bind close_cb so that to state so that within it, self will + " explicitly bind close_cb to state so that within it, self will " always refer to state. See :help Partial for more information. let cbs.close_cb = function('s:close_cb', [], state) diff --git a/autoload/go/term.vim b/autoload/go/term.vim index 23ee18c47b..c513a4e876 100644 --- a/autoload/go/term.vim +++ b/autoload/go/term.vim @@ -2,9 +2,6 @@ if has('nvim') && !exists("g:go_term_mode") let g:go_term_mode = 'vsplit' endif -" s:jobs is a global reference to all jobs started with new() -let s:jobs = {} - " new creates a new terminal with the given command. Mode is set based on the " global variable g:go_term_mode, which is by default set to :vsplit function! go#term#new(bang, cmd) abort @@ -18,8 +15,15 @@ function! go#term#newmode(bang, cmd, mode) abort let mode = g:go_term_mode endif + let state = { + \ 'cmd': a:cmd, + \ 'bang' : a:bang, + \ 'winnr': winnr(), + \ 'stdout': [], + \ 'stderr': [] + \ } + " execute go build in the files directory - let l:winnr = winnr() let cd = exists('*haslocaldir') && haslocaldir() ? 'lcd ' : 'cd ' let dir = getcwd() @@ -33,30 +37,27 @@ function! go#term#newmode(bang, cmd, mode) abort setlocal noswapfile setlocal nobuflisted + " explicitly bind callbacks to state so that within them, self will always + " refer to state. See :help Partial for more information. let job = { - \ 'stderr' : [], - \ 'stdout' : [], - \ 'bang' : a:bang, - \ 'on_stdout': function('s:on_stdout'), - \ 'on_stderr': function('s:on_stderr'), - \ 'on_exit' : function('s:on_exit'), + \ 'on_stdout': function('s:on_stdout', [], state), + \ 'on_stderr': function('s:on_stderr', [], state), + \ 'on_exit' : function('s:on_exit', [], state), \ } - let id = termopen(a:cmd, job) + let state.id = termopen(a:cmd, job) + let state.termwinnr = winnr() execute cd . fnameescape(dir) - let job.id = id - let job.cmd = a:cmd startinsert " resize new term if needed. let height = get(g:, 'go_term_height', winheight(0)) let width = get(g:, 'go_term_width', winwidth(0)) - " we are careful how to resize. for example it's vsplit we don't change - " the height. The below command resizes the buffer - + " Adjust the window width or height depending on whether it's a vertical or + " horizontal split. if mode =~ "vertical" || mode =~ "vsplit" || mode =~ "vnew" exe 'vertical resize ' . width elseif mode =~ "split" || mode =~ "new" @@ -64,77 +65,63 @@ function! go#term#newmode(bang, cmd, mode) abort endif " we also need to resize the pty, so there you go... - call jobresize(id, width, height) + call jobresize(state.id, width, height) - let s:jobs[id] = job stopinsert - if l:winnr !=# winnr() - exe l:winnr . "wincmd w" + if state.winnr !=# winnr() + exe state.winnr . "wincmd w" endif - return id + return state.id endfunction function! s:on_stdout(job_id, data, event) dict abort - if !has_key(s:jobs, a:job_id) - return - endif - let job = s:jobs[a:job_id] - - call extend(job.stdout, a:data) + call extend(self.stdout, a:data) endfunction function! s:on_stderr(job_id, data, event) dict abort - if !has_key(s:jobs, a:job_id) - return - endif - let job = s:jobs[a:job_id] - - call extend(job.stderr, a:data) + call extend(self.stderr, a:data) endfunction function! s:on_exit(job_id, exit_status, event) dict abort - if !has_key(s:jobs, a:job_id) - return - endif - let job = s:jobs[a:job_id] - let l:listtype = go#list#Type("_term") " usually there is always output so never branch into this clause - if empty(job.stdout) + if empty(self.stdout) call go#list#Clean(l:listtype) call go#list#Window(l:listtype) - unlet s:jobs[a:job_id] return endif - let errors = go#tool#ParseErrors(job.stdout) + let errors = go#tool#ParseErrors(self.stdout) let errors = go#tool#FilterValids(errors) if !empty(errors) - " close terminal we don't need it anymore - close + " close terminal; we don't need it anymore + execute self.termwinnr . "close" + + if self.winnr !=# winnr() + execute self.winnr . "wincmd w" + endif - call go#list#Populate(l:listtype, errors, job.cmd) + call go#list#Populate(l:listtype, errors, self.cmd) call go#list#Window(l:listtype, len(errors)) if !self.bang call go#list#JumpToFirst(l:listtype) endif - unlet s:jobs[a:job_id] + return endif - " tests are passing clean the list and close the list. But we only can - " close them from a normal view, so jump back, close the list and then - " again jump back to the terminal - wincmd p - call go#list#Clean(l:listtype) - call go#list#Window(l:listtype) - wincmd p - - unlet s:jobs[a:job_id] + " There are no errors. Clean and close the list. Jump to the window to which + " the location list is attached, close the list, and then jump back to the + " current window. + let l:winnr = winnr() + execute self.winnr . "wincmd w" + call go#list#Clean(l:listtype) + call go#list#Window(l:listtype) + execute l:winnr . "wincmd w" endfunction " vim: sw=2 ts=2 et