Allow arguments for :Go{Install,Update}Binaries

[arp242]

• Support arguments; fixes Add :GoUpdateBinaries binaryName support #1438
• Add -ldflags -H=windowsgui when installing gocode on Windows; fixes gocode installation on Windows with GoInstallBinaries breaks autocomplete #1454

[bhcleek]

This is a welcome change. Good work @Carpetsmoker.

In the edited docs

If one or more [binaries] are given

reads as if the user can provide zero or more lists, but it's in fact a single optional list. There are other examples in the vim-go docs that we can use for a starting point. Perhaps

If {binaries} is supplied, then only the specified binaries will be ....

I put a couple of other suggestions inline, but feel free to take those or not; they're just suggestions.

[bhcleek]

I get why pkg[0] and pkg[1] are used here, but it it's pretty far from s:packages. For readability, it's probably worth naming giving them meaningful names by assigning their values to variables at the start of the for loop. Something like:

let l:pkgpath = pkg[0]
let l:gogetflags = (len(pkg) > 1 ? get(pkg[1], l:platform, '') : '')

And then use those instead of pkg[0] and pkg[1].

[arp242]

Thanks; updated!

[bhcleek]

Do we really want a dictionary here? In every case the key is simply the basename of the first element in the list.

Perhaps this would suffice:

let s:packages = {
    \  'github.com/klauspost/asmfmt/cmd/asmfmt': {}`,
...
    \ 'github.com/nsf/gocode': {'windows: '-ldflags -H=windowsgui'},
...
\  }

Food for thought...

[arp242]

I started some work on converting this, but it complicates some of the logic by quite a bit (e.g. for filtering the selected packages). I think that keeping it like this is better (a little bit of redundancy in data is better than complicated logic).

I saved the (non-working) patch here: https://gist.github.com/Carpetsmoker/f2003d47f985734c766dff6b68390717

[bhcleek]

LGTM, but I think you can still simplify the dictionary somewhat. Here's a PoC that's functionally closer to your current implementation, but without the need for an additional function to filter:

diff --git a/plugin/go.vim b/plugin/go.vim
index 74b74bb..83ad0c3 100644
--- a/plugin/go.vim
+++ b/plugin/go.vim
@@ -7,22 +7,22 @@ let g:go_loaded_install = 1
 " these packages are used by vim-go and can be automatically installed if
 " needed by the user with GoInstallBinaries
 let s:packages = {
-      \ 'asmfmt':        ['github.com/klauspost/asmfmt/cmd/asmfmt'],
-      \ 'errcheck':      ['github.com/kisielk/errcheck'],
-      \ 'fillstruct':    ['github.com/davidrjenni/reftools/cmd/fillstruct'],
-      \ 'gocode':        ['github.com/nsf/gocode', {'windows': '-ldflags -H=windowsgui'}],
-      \ 'godef':         ['github.com/rogpeppe/godef'],
-      \ 'gogetdoc':      ['github.com/zmb3/gogetdoc'],
-      \ 'goimports':     ['golang.org/x/tools/cmd/goimports'],
-      \ 'golint':        ['github.com/golang/lint/golint'],
-      \ 'gometalinter':  ['github.com/alecthomas/gometalinter'],
-      \ 'gomodifytags':  ['github.com/fatih/gomodifytags'],
-      \ 'gorename':      ['golang.org/x/tools/cmd/gorename'],
-      \ 'gotags':        ['github.com/jstemmer/gotags'],
-      \ 'guru':          ['golang.org/x/tools/cmd/guru'],
-      \ 'impl':          ['github.com/josharian/impl'],
-      \ 'keyify':        ['github.com/dominikh/go-tools/cmd/keyify'],
-      \ 'motion':        ['github.com/fatih/motion'],
+      \ 'github.com/klauspost/asmfmt/cmd/asmfmt':          {},
+      \ 'github.com/kisielk/errcheck':                     {},
+      \ 'github.com/davidrjenni/reftools/cmd/fillstruct':  {},
+      \ 'github.com/nsf/gocode':                           {'windows':  '-ldflags -H=windowsgui'},
+      \ 'github.com/rogpeppe/godef':                       {},
+      \ 'github.com/zmb3/gogetdoc':                        {},
+      \ 'golang.org/x/tools/cmd/goimports':                {},
+      \ 'github.com/golang/lint/golint':                   {},
+      \ 'github.com/alecthomas/gometalinter':              {},
+      \ 'github.com/fatih/gomodifytags':                   {},
+      \ 'golang.org/x/tools/cmd/gorename':                 {},
+      \ 'github.com/jstemmer/gotags':                      {},
+      \ 'golang.org/x/tools/cmd/guru':                     {},
+      \ 'github.com/josharian/impl':                       {},
+      \ 'github.com/dominikh/go-tools/cmd/keyify':         {},
+      \ 'github.com/fatih/motion':                         {},
 \ }
 
 " These commands are available on any filetypes
@@ -30,8 +30,10 @@ command! -nargs=* -complete=customlist,s:complete GoInstallBinaries call s:GoIns
 command! -nargs=* -complete=customlist,s:complete GoUpdateBinaries  call s:GoInstallBinaries(1, <f-args>)
 command! -nargs=? -complete=dir GoPath call go#path#GoPath(<f-args>)
 
-fun! s:complete(lead, cmdline, cursor)
-  return filter(keys(s:packages), 'strpart(v:val, 0, len(a:lead)) == a:lead')
+function! s:complete(lead, cmdline, cursor)
+  return filter(
+        \ map(keys(s:packages), 'fnamemodify(v:val, ":t")'),
+        \ 'strpart(v:val, 0, len(a:lead)) == a:lead')
 endfun
 
 " GoInstallBinaries downloads and install all necessary binaries stated in the
@@ -88,12 +90,18 @@ function! s:GoInstallBinaries(updateBinaries, ...)
   let l:packages = {}
   if a:0 > 0
     for l:bin in a:000
-      let l:pkg = get(s:packages, l:bin, [])
-      if len(l:pkg) == 0
+      let l:binmatch = filter(
+            \ map(keys(s:packages), 'fnamemodify(v:val, ":t")'),
+            \ 'v:val == l:bin'
+            \ )
+      if len(l:binmatch) == 0
         call go#util#EchoError('unknown binary: ' . l:bin)
         return
       endif
-      let l:packages[l:bin] = l:pkg
+      if len(l:binmatch) > 1
+        call go#util#EchoWarning("multiple packages match " . l:bin)
+      endif
+      let l:packages[l:bin] = l:binmatch[0]
     endfor
   else
     let l:packages = s:packages

If you want to merge as is, then feel free, but perhaps this PoC clears the way for some simplification for you?