Improve debugger

autoload/go/config.vim

@@ -206,7 +206,7 @@ endfunction
 
 function! go#config#DebugCommands() abort
   " make sure g:go_debug_commands is set so that it can be added to easily.
-  let g:go_debug_commands = get(g:, 'go_debug_commands', {})
+  let g:go_debug_commands = get(g:, 'go_debug_commands', [])
   return g:go_debug_commands
 endfunction
 

autoload/go/debug.vim

@@ -71,81 +71,40 @@ function! s:call_jsonrpc(method, ...) abort
     echom 'sending to dlv ' . a:method
   endif
 
-  if len(a:000) > 0 && type(a:000[0]) == v:t_func
-     let Cb = a:000[0]
-     let args = a:000[1:]
-  else
-     let args = a:000
-  endif
+  let l:args = a:000
   let s:state['rpcid'] += 1
-  let req_json = json_encode({
+  let l:req_json = json_encode({
       \  'id': s:state['rpcid'],
       \  'method': a:method,
-      \  'params': args,
+      \  'params': l:args,
       \})
 
   try
-    " Use callback
-    if exists('l:Cb')
-      if has('nvim')
-        let state = {'callback': Cb}
-        function! state.on_data(ch, msg, event) abort
-          call self.state.callback(a:ch, a:msg)
-        endfunction
-        let l:ch = sockconnect('tcp', go#config#DebugAddress(), {'on_data': state.on_data, 'state': state})
-        call chansend(l:ch, req_json)
-
-        if go#util#HasDebug('debugger-commands')
-          let g:go_debug_commands = add(g:go_debug_commands, {
-                \ 'request':  req_json,
-                \ 'response': Cb,
-          \ })
-        endif
-        return
-      endif
-
-      let l:ch = ch_open(go#config#DebugAddress(), {'mode': 'nl', 'callback': Cb})
-      call ch_sendraw(l:ch, req_json)
-
-      if go#util#HasDebug('debugger-commands')
-        let g:go_debug_commands = add(g:go_debug_commands, {
-              \ 'request':  req_json,
-              \ 'response': Cb,
-        \ })
-      endif
-      return
-    endif
-
+    let l:ch = s:state['ch']
     if has('nvim')
-      let state = {'done': 0}
-      function! state.on_data(ch, msg, event) abort
-        let self.state.resp = a:msg
-        let self.state.done = 1
-      endfunction
-      let l:ch = sockconnect('tcp', go#config#DebugAddress(), {'on_data': state.on_data, 'state': state})
-      call chansend(l:ch, req_json)
-      while state.done == 0
+      call chansend(l:ch, l:req_json)
+      while len(s:state.data) == 0
         sleep 50m
       endwhile
-      let resp_json = state.resp
+      let resp_json = s:state.data[0]
+      let s:state.data = s:state.data[1:]
     else
-      let ch = ch_open(go#config#DebugAddress(), {'mode': 'raw', 'timeout': 20000})
-      call ch_sendraw(ch, req_json)
-      let resp_json = ch_readraw(ch)
+      call ch_sendraw(l:ch, req_json)
+      let l:resp_raw = ch_readraw(l:ch)
+      let resp_json = json_decode(l:resp_raw)
     endif
 
     if go#util#HasDebug('debugger-commands')
-      let g:go_debug_commands = add(g:go_debug_commands, {
-            \ 'request':  req_json,
-            \ 'response': resp_json,
+      let g:go_debug_commands = add(go#config#DebugCommands(), {
+            \ 'request':  l:req_json,
+            \ 'response': l:resp_json,
       \ })
     endif
 
-    let obj = json_decode(resp_json)
-    if type(obj) == v:t_dict && has_key(obj, 'error') && !empty(obj.error)
-      throw obj.error
+    if type(l:resp_json) == v:t_dict && has_key(l:resp_json, 'error') && !empty(l:resp_json.error)
+      throw l:resp_json.error
     endif
-    return obj
+    return l:resp_json
   catch
     throw substitute(v:exception, '^Vim', '', '')
   endtry
@@ -251,12 +210,12 @@ function! s:clearState() abort
   let s:state['localVars'] = {}
   let s:state['functionArgs'] = {}
   let s:state['message'] = []
+
   silent! sign unplace 9999
 endfunction
 
 function! s:stop() abort
-  " TODO(bc): call Detach
-  call go#job#Stop(s:state['job'])
+  let l:res = s:call_jsonrpc('RPCServer.Detach', {'kill': v:true})
 
   call s:clearState()
   if has_key(s:state, 'job')
@@ -266,9 +225,19 @@ function! s:stop() abort
   if has_key(s:state, 'ready')
     call remove(s:state, 'ready')
   endif
+
+  if has_key(s:state, 'ch')
+    call remove(s:state, 'ch')
+  endif
+
+  if has_key( s:state, 'data')
+    call remove(s:state, 'data')
+  endif
 endfunction
 
 function! go#debug#Stop() abort
+  " TODO(bc): don't remove breakpoints that were set before debugging started
+  " (see out_cb).
   " Remove signs.
   for k in keys(s:state['breakpoint'])
     let bt = s:state['breakpoint'][k]
@@ -430,15 +399,11 @@ function! s:expand_var() abort
   endif
 endfunction
 
-function! s:start_cb(ch, json) abort
-  let res = json_decode(a:json)
-  if type(res) == v:t_dict && has_key(res, 'error') && !empty(res.error)
-    throw res.error
-  endif
-  if empty(res) || !has_key(res, 'result')
+function! s:start_cb(res) abort
+  if empty(a:res) || !has_key(a:res, 'result')
     return
   endif
-  for bt in res.result.Breakpoints
+  for bt in a:res.result.Breakpoints
     if bt.id >= 0
       let s:state['breakpoint'][bt.id] = bt
       exe 'sign place '. bt.id .' line=' . bt.line . ' name=godebugbreakpoint file=' . bt.file
@@ -538,6 +503,46 @@ function! s:out_cb(ch, msg) abort
   let s:state['message'] += [a:msg]
 
   if stridx(a:msg, go#config#DebugAddress()) != -1
+    if has('nvim')
+      let s:state['data'] = []
+      let l:state = {'databuf': ''}
+      function! s:on_data(ch, data, event) dict abort closure
+        let l:data = self.databuf
+        for msg in a:data
+          let l:data .= l:msg
+        endfor
+
+        try
+          let l:res = json_decode(l:data)
+          let s:state['data'] = add(s:state['data'], l:res)
+          let self.databuf = ''
+        catch
+          " there isn't a complete message in databuf: buffer l:data and try
+          " again when more data comes in.
+          let self.databuf = l:data
+        finally
+        endtry
+      endfunction
+      " explicitly bind callback to state so that within it, self will
+      " always refer to state. See :help Partial for more information.
+      let l:state.on_data = function('s:on_data', [], l:state)
+      let l:ch = sockconnect('tcp', go#config#DebugAddress(), {'on_data': l:state.on_data, 'state': l:state})
+      if l:ch == 0
+        call go#util#EchoError("could not connect to debugger")
+        call go#job#Stop(s:state['job'])
+        return
+      endif
+    else
+      let l:ch = ch_open(go#config#DebugAddress(), {'mode': 'raw', 'timeout': 20000})
+      if ch_status(l:ch) !=# 'open'
+        call go#util#EchoError("could not connect to debugger")
+        call go#job#Stop(s:state['job'])
+        return
+      endif
+    endif
+
+    let s:state['ch'] = l:ch
+
     " After this block executes, Delve will be running with all the
     " breakpoints setup, so this callback doesn't have to run again; just log
     " future messages.
@@ -550,7 +555,8 @@ function! s:out_cb(ch, msg) abort
       call go#debug#Breakpoint(bt.line)
     endfor
 
-    call s:call_jsonrpc('RPCServer.ListBreakpoints', function('s:start_cb'))
+    let res = s:call_jsonrpc('RPCServer.ListBreakpoints')
+    call s:start_cb(res)
   endif
 endfunction
 
@@ -575,13 +581,6 @@ function! go#debug#Start(is_test, ...) abort
     call go#config#SetDebugDiag(s:state)
   endif
 
-  " cd in to test directory; this is also what running "go test" does.
-  if a:is_test
-    lcd %:p:h
-  endif
-
-  let s:state.is_test = a:is_test
-
   let dlv = go#path#CheckBinPath("dlv")
   if empty(dlv)
     return
@@ -590,14 +589,27 @@ function! go#debug#Start(is_test, ...) abort
   try
     if len(a:000) > 0
       let l:pkgname = a:1
-      " Expand .; otherwise this won't work from a tmp dir.
       if l:pkgname[0] == '.'
-        let l:pkgname = go#package#FromPath(getcwd()) . l:pkgname[1:]
+        let l:pkgname = go#package#FromPath(l:pkgname)
       endif
     else
       let l:pkgname = go#package#FromPath(getcwd())
     endif
 
+    if l:pkgname is -1
+      call go#util#EchoError('could not determine package name')
+      return
+    endif
+
+    " cd in to test directory; this is also what running "go test" does.
+    if a:is_test
+      " TODO(bc): Either remove this if it's ok to do so or else record it and
+      " reset cwd after the job completes.
+      lcd %:p:h
+    endif
+
+    let s:state.is_test = a:is_test
+
     let l:args = []
     if len(a:000) > 1
       let l:args = ['--'] + a:000[1:]
@@ -612,7 +624,6 @@ function! go#debug#Start(is_test, ...) abort
           \ '--api-version', '2',
           \ '--log', '--log-output', 'debugger,rpc',
           \ '--listen', go#config#DebugAddress(),
-          \ '--accept-multiclient',
     \]
 
     let buildtags = go#config#BuildTags()
@@ -807,20 +818,13 @@ function! s:update_stacktrace() abort
   endtry
 endfunction
 
-function! s:stack_cb(ch, json) abort
+function! s:stack_cb(res) abort
   let s:stack_name = ''
-  let res = json_decode(a:json)
-  if type(res) == v:t_dict && has_key(res, 'error') && !empty(res.error)
-    call go#util#EchoError(res.error)
-    call s:clearState()
-    call go#debug#Restart()
-    return
-  endif
 
-  if empty(res) || !has_key(res, 'result')
+  if empty(a:res) || !has_key(a:res, 'result')
     return
   endif
-  call s:update_breakpoint(res)
+  call s:update_breakpoint(a:res)
   call s:update_stacktrace()
   call s:update_variables()
 endfunction
@@ -851,7 +855,14 @@ function! go#debug#Stack(name) abort
       call s:call_jsonrpc('RPCServer.CancelNext')
     endif
     let s:stack_name = l:name
-    call s:call_jsonrpc('RPCServer.Command', function('s:stack_cb'), {'name': l:name})
+    try
+      let res =  s:call_jsonrpc('RPCServer.Command', {'name': l:name})
+      call s:stack_cb(res)
+    catch
+      call go#util#EchoError(v:exception)
+      call s:clearState()
+      call go#debug#Restart()
+    endtry
   catch
     call go#util#EchoError(v:exception)
   endtry

autoload/go/debug_test.vim

@@ -0,0 +1,54 @@
+" don't spam the user when Vim is started in Vi compatibility mode
+let s:cpo_save = &cpo
+set cpo&vim
+
+function! Test_GoDebugStart_Empty() abort
+  call s:debug()
+endfunction
+
+function! Test_GoDebugStart_RelativePackage() abort
+  call s:debug('./debugmain')
+endfunction
+
+function! Test_GoDebugStart_Package() abort
+  call s:debug('debugmain')
+endfunction
+
+function! s:debug(...) abort
+  if !go#util#has_job()
+    return
+  endif
+
+  try
+    let l:tmp = gotest#load_fixture('debugmain/debugmain.go')
+
+    call go#debug#Breakpoint(6)
+
+    call assert_false(exists(':GoDebugStop'))
+
+    if a:0 == 0
+      let l:cd = exists('*haslocaldir') && haslocaldir() ? 'lcd' : 'cd'
+      execute l:cd . ' debugmain'
+      call go#debug#Start(0)
+    else
+      call go#debug#Start(0, a:1)
+    endif
+
+    let l:start = reltime()
+    while !exists(':GoDebugStop') && reltimefloat(reltime(l:start)) < 10
+      sleep 100m
+    endwhile
+
+    call go#debug#Stop()
+
+    call assert_false(exists(':GoDebugStop'))
+  finally
+    call delete(l:tmp, 'rf')
+  endtry
+endfunction
+
+" restore Vi compatibility settings
+let &cpo = s:cpo_save
+unlet s:cpo_save
+
+" vim: sw=2 ts=2 et

autoload/go/job.vim

@@ -92,7 +92,6 @@ function! go#job#Options(args)
     let state.errorformat = a:args.errorformat
   endif
 
-  " do nothing in state.complete by default.
   function state.complete(job, exit_status, data)
     if has_key(self, 'custom_complete')
       let l:winid = win_getid(winnr())