code review 5699079: misc: add zsh completion for go tool.

misc: add zsh completion for go tool.

[remyoudompheng, 2012-02-26 15:27:13]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[minux1, 2012-02-26 15:37:53]

FYI, http://codereview.appspot.com/5646066/

[remyoudompheng, 2012-02-26 15:38:30]

Completion is slow because it calls "go list prefix..." each time. Another
solution could be to use directory paths from $GOROOT/src/pkg and $GOPATH/src
(less accurate but probably faster), but I'm not yet sure how to do that.

[remyoudompheng, 2012-02-26 15:40:55]

On 2012/02/26 15:37:53, minux wrote:
> FYI, http://codereview.appspot.com/5646066/

Oh, I didn't see that. That leaves zsh completion unless I also missed a CL
concerning this.

[remyoudompheng, 2012-02-27 07:14:16]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[remyoudompheng, 2012-02-28 00:18:22]

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go#newcode22
misc/zsh/go:22: }
I also propose the following code:

__godirs() {
  local gopaths="${(s/:/)GOPATH}"
  local goroot=$(go tool dist env | sed '/GOROOT=/s,GOROOT=,,; s/"//g; q')
  echo "$goroot/src/pkg" "${gopaths[@]//%//src}"
}

compctl -x \
  "p[1]" -k "(build clean doc fix fmt get install list run test tool version vet
help)" \
  - "W[1,clean|doc|fix|fmt|install|list|test|vet]" -/ -W "($(__godirs))" \
  - "w[1,build]" -g "*.go" -/ -W "($(__godirs))" \

[...]

don't know which one sounds better.

[rsc, 2012-02-28 15:45:41]

I have no idea about any of this.
I suggest that minux review this CL
and that remy review CL 5646066,
and that's good enough for me.  :-)

Russ

[minux1, 2012-02-28 16:50:09]

I've tried this CL, and it's very helpful. Some small suggestions.

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go#newcode7
misc/zsh/go:7: compctl -g "*.go" ${p}g
We use 'go tool 6g' now, so these is not needed.
But, I wonder, if someone adds $GOROOT/pkg/tool/$GOOS_$GOARCH/
to his $PATH, then these are indeed useful. (Of course, this
isn't  recommended, because it will cause some name collision.)

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go#newcode22
misc/zsh/go:22: }
On 2012/02/28 00:18:23, remyoudompheng wrote:
> I also propose the following code:
> 
> __godirs() {
>   local gopaths="${(s/:/)GOPATH}"
>   local goroot=$(go tool dist env | sed '/GOROOT=/s,GOROOT=,,; s/"//g; q')
>   echo "$goroot/src/pkg" "${gopaths[@]//%//src}"
> }
> 
> compctl -x \
>   "p[1]" -k "(build clean doc fix fmt get install list run test tool version
vet
> help)" \
>   - "W[1,clean|doc|fix|fmt|install|list|test|vet]" -/ -W "($(__godirs))" \
>   - "w[1,build]" -g "*.go" -/ -W "($(__godirs))" \
> 
> [...]
> 
> don't know which one sounds better.
I think this approach is faster, but it is static, and can't handle
the case where the user changes GOPATH after sourcing this file.

The other problem with this approach is that it is easy to trigger
issue 3034 when the user issued 'go test archive/tar/'.

http://codereview.appspot.com/5699079/diff/3003/misc/zsh/go#newcode27
misc/zsh/go:27: - "w[1,build]" -g "*.go" -K __go_list \
Consider add support for basic go build/go test option flags?
(Although I think this can be addressed by subsequent CLs)

[remyoudompheng, 2012-02-28 19:16:06]

Completing one-letter option names does not really seem useful unless a
description is also printed. I'll rewrite the CL using the newer zsh completion
sytem.

[minux1, 2012-02-28 19:26:08]

On Wed, Feb 29, 2012 at 3:16 AM, <remyoudompheng@gmail.com> wrote:

> Completing one-letter option names does not really seem useful unless a
> description is also printed.

Yes, I mean that.
Might be able to automatically generate some of the completion
configurations
from cmd/go source code, so that we won't be out-of-date.

> I'll rewrite the CL using the newer zsh
> completion sytem.
>
Great.

[remyoudompheng, 2012-02-28 20:36:39]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[remyoudompheng, 2012-03-01 19:59:49]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[minux1, 2012-03-01 20:43:21]

Sorry for the delayed review.

Very impressive work, LGTM, thanks.

http://codereview.appspot.com/5699079/diff/13001/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/13001/misc/zsh/go#newcode43
misc/zsh/go:43: '-x[print the commands]'
-p n
-work # (this one is pretty important when diagnosing problems)
-gccgoflags 'arg list'  # these four are added recently
-gcflags 'arg list'
-ldflags 'flag list'
-tags 'tag list'

http://codereview.appspot.com/5699079/diff/13001/misc/zsh/go#newcode67
misc/zsh/go:67: ${build_flags[@]} \
'go test' has its own list of flags, not all 'go build' flags is applicable to
it.

[remyoudompheng, 2012-03-01 20:54:06]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[minux1, 2012-03-01 21:01:52]

Does 'go test' support -a?

http://codereview.appspot.com/5699079/diff/17001/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/17001/misc/zsh/go#newcode78
misc/zsh/go:78: ${build_flags[@]} \
-i and -c.

[remyoudompheng, 2012-03-01 21:07:57]

On 2012/03/01 21:01:52, minux wrote:
> Does 'go test' support -a?

The documentation (go help build) says it does, but it doesn't work. Same thing
for -work etc.
 
> http://codereview.appspot.com/5699079/diff/17001/misc/zsh/go
> File misc/zsh/go (right):
> 
> http://codereview.appspot.com/5699079/diff/17001/misc/zsh/go#newcode78
> misc/zsh/go:78: ${build_flags[@]} \
> -i and -c.

Done.

[remyoudompheng, 2012-03-01 21:20:04]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[rsc, 2012-03-01 23:03:37]

On Thu, Mar 1, 2012 at 16:07,  <remyoudompheng@gmail.com> wrote:
>> Does 'go test' support -a?
>
> The documentation (go help build) says it does, but it doesn't work.
> Same thing for -work etc.

let's fix that (separate CL)

[minux1, 2012-03-02 17:20:09]

LGTM.

http://codereview.appspot.com/5699079/diff/15004/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/15004/misc/zsh/go#newcode77
misc/zsh/go:77: _arguments -s -w : \
According to rsc, you can still use "${build_flags[@]}" here, and avoid
the duplication.

[remyoudompheng, 2012-03-02 17:35:38]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[minux1, 2012-03-02 17:42:55]

http://codereview.appspot.com/5699079/diff/23001/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/23001/misc/zsh/go#newcode83
misc/zsh/go:83: "-i[do not run, install dependencies]" \
why delete -v?

http://codereview.appspot.com/5699079/diff/23001/misc/zsh/go#newcode91
misc/zsh/go:91: "-memprofile[write heap profile to file]:file:_files" \
-timeout t
-memprofilerate n
-benchtime

(refer to variable testFlagDefn in cmd/go/testflag.go for complete list)

[remyoudompheng, 2012-03-02 17:46:58]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[minux1, 2012-03-02 19:48:04]

LGTM. No more suggestions :-)

[remyoudompheng, 2012-03-02 23:13:06]

*** Submitted as http://code.google.com/p/go/source/detail?r=de155800afc1 ***

misc: add zsh completion for go tool.

R=golang-dev, minux.ma, rsc
CC=golang-dev, remy
http://codereview.appspot.com/5699079

http://codereview.appspot.com/5699079/diff/13001/misc/zsh/go
File misc/zsh/go (right):

http://codereview.appspot.com/5699079/diff/13001/misc/zsh/go#newcode43
misc/zsh/go:43: '-x[print the commands]'
"build_flags" is the list of flags common to build/get/install/test, i can add
the flags below