Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1113)

Issue 99790043: code review 99790043: cmd/gofmt: add -i flag for blacklisting certain -r wildcards

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by joeshaw
Modified:
9 years, 9 months ago
CC:
golang-codereviews
Visibility:
Public.

Description

cmd/gofmt: add -i flag for blacklisting certain -r wildcards It is idiomatic to use single lower case identifiers in Go, but this can lead to conflicts when attempting to rewrite code, as gofmt -r treats them as wildcards in the pattern portion of the flag. This new -i flag treats the provided values as literal identifiers rather than wildcards. Fixes issue 7870.

Patch Set 1 #

Patch Set 2 : diff -r d4d38f1aa322 https://code.google.com/p/go/ #

Patch Set 3 : diff -r d4d38f1aa322 https://code.google.com/p/go/ #

Patch Set 4 : diff -r d4d38f1aa322 https://code.google.com/p/go/ #

Patch Set 5 : diff -r f39dff1bf0d4 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -6 lines) Patch
M src/cmd/gofmt/gofmt.go View 1 1 chunk +7 lines, -6 lines 0 comments Download
M src/cmd/gofmt/gofmt_test.go View 1 2 chunks +3 lines, -0 lines 0 comments Download
M src/cmd/gofmt/rewrite.go View 1 2 chunks +11 lines, -0 lines 0 comments Download
A src/cmd/gofmt/testdata/rewrite9.golden View 1 1 chunk +11 lines, -0 lines 0 comments Download
A src/cmd/gofmt/testdata/rewrite9.input View 1 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 9
joeshaw
I'd been bit by this a few times in the past, and it seemed like ...
9 years, 11 months ago (2014-04-26 17:49:44 UTC) #1
joeshaw
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
9 years, 11 months ago (2014-04-26 17:50:05 UTC) #2
gobot
R=rsc@golang.org (assigned by r@golang.org)
9 years, 11 months ago (2014-04-29 14:40:07 UTC) #3
rsc
R=close We're in a release freeze right now. Please run 'hg mail' after Go 1.3 ...
9 years, 11 months ago (2014-05-02 15:57:00 UTC) #4
joeshaw
Hello golang-codereviews@googlegroups.com, gobot@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
9 years, 9 months ago (2014-06-19 14:57:36 UTC) #5
joeshaw
Hello golang-codereviews@googlegroups.com, gobot@golang.org, rsc@golang.org (cc: golang-codereviews@googlegroups.com), Please take another look.
9 years, 9 months ago (2014-06-19 15:03:41 UTC) #6
rsc
not lgtm I'm sorry, but I think this is pushing gofmt too far toward something ...
9 years, 9 months ago (2014-06-19 15:43:11 UTC) #7
rsc
R=close
9 years, 9 months ago (2014-06-19 15:43:16 UTC) #8
joeshaw
9 years, 9 months ago (2014-06-19 16:22:09 UTC) #9
On 2014/06/19 15:43:11, rsc wrote:
> not lgtm
> 
> I'm sorry, but I think this is pushing gofmt too far toward something it is
not.
> -r was cute but isn't the primary role, and I may have been wrong to add it.
> Expanding its role seems like going in the wrong direction.
> 
> I think that if you need more than -r gives, it makes sense to create a
separate
> tool rather than keep expanding gofmt. One way to do this would be to copy the
> gofmt source somewhere and modify it. That's fine.
> 
> Also, Alan Donovan has been working on a new refactoring tool:
>    go get code.google.com/p/go.tools/cmd/eg
> 
> https://code.google.com/p/go/source/browse/refactor/eg/eg.go?repo=tools has a
> large comment about how to use it.

Ok, I'll look into that.  Thanks for looking this patch set over anyway.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b