code review 4634076: gofix: fixes for sort changes

gofix: fixes for sort changes

Fixes issue 1969.

[rh, 2011-06-22 04:27:44]

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

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

[gri1, 2011-06-22 19:41:34]

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go
File src/cmd/gofix/sortslice.go (right):

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:30: var newName string
please move this declaration closer to its use, i.e., just before the switch
statement

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:47: s.Sel = ast.NewIdent(newName)
s.Sel.Name = newName

(I would also s/newName/name/ but I leave that to your preference)

(this way the AST is not losing position information stored in the identifier -
also, note that identifies are not shared, so this is ok).

[rsc, 2011-06-22 20:06:26]

thanks for this one too!

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/Makefile
File src/cmd/gofix/Makefile (right):

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/Makefile#newcode19
src/cmd/gofix/Makefile:19: sortslice.go\
move up one line (sort)
:-)

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go
File src/cmd/gofix/sortslice.go (right):

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:36: switch s.Sel.String() {
A simpler thing to do is just

case "Float64Array":
    s.Sel.Name = "Float64Slice"
...

[rh, 2011-06-23 00:34:38]

Thank you for the reviews.  PTAL.

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/Makefile
File src/cmd/gofix/Makefile (right):

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/Makefile#newcode19
src/cmd/gofix/Makefile:19: sortslice.go\
On 2011/06/22 20:06:27, rsc wrote:
> move up one line (sort)
> :-)

Done. (sorted the whole thing... netdial was before main)

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go
File src/cmd/gofix/sortslice.go (right):

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:30: var newName string
On 2011/06/22 19:41:35, gri1 wrote:
> please move this declaration closer to its use, i.e., just before the switch
> statement

I tried moving it down, but it fell off the bottom of the file.

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:36: switch s.Sel.String() {
On 2011/06/22 20:06:27, rsc wrote:
> A simpler thing to do is just
> 
> case "Float64Array":
>     s.Sel.Name = "Float64Slice"
> ...

Done.

http://codereview.appspot.com/4634076/diff/4001/src/cmd/gofix/sortslice.go#ne...
src/cmd/gofix/sortslice.go:47: s.Sel = ast.NewIdent(newName)
On 2011/06/22 19:41:35, gri1 wrote:
> s.Sel.Name = newName
> 
> (I would also s/newName/name/ but I leave that to your preference)
> 
> (this way the AST is not losing position information stored in the identifier
-
> also, note that identifies are not shared, so this is ok).

Done. (merged into switch above)

[r, 2011-06-25 22:45:44]

LGTM

[r, 2011-06-25 22:49:06]

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

gofix: fixes for sort changes

Fixes issue 1969.

R=gri, rsc, r
CC=golang-dev
http://codereview.appspot.com/4634076

Committer: Rob Pike <r@golang.org>