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

Issue 4661051: code review 4661051: strings.Split: make the default to split all. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 10 months ago by r
Modified:
1 year, 3 months ago
CC:
adg, dsymonds, brainman, rsc, golang-dev
Visibility:
Public.

Description

strings.Split: make the default to split all. Change the signature of Split to have no count, assuming a full split, and rename the existing Split with a count to SplitN. Do the same to package bytes. Add a gofix module.

Patch Set 1 #

Patch Set 2 : diff -r b295b8bda20b https://go.googlecode.com/hg/ #

Total comments: 7

Patch Set 3 : diff -r b295b8bda20b https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 4 : diff -r b295b8bda20b https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r eaa696629d4d https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+280 lines, -104 lines) Patch
M misc/dashboard/builder/main.go View 1 4 chunks +4 lines, -4 lines 0 comments Download
M src/cmd/cgo/gcc.go View 1 5 chunks +5 lines, -5 lines 0 comments Download
M src/cmd/cgo/out.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/godoc/dirtrees.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/cmd/godoc/godoc.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/godoc/index.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/gofix/Makefile View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/gofix/main.go View 1 1 chunk +1 line, -1 line 0 comments Download
A src/cmd/gofix/stringssplit.go View 1 2 3 1 chunk +71 lines, -0 lines 0 comments Download
A src/cmd/gofix/stringssplit_test.go View 1 1 chunk +51 lines, -0 lines 0 comments Download
M src/cmd/gofix/testdata/reflect.template.go.in View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/cmd/gofix/testdata/reflect.template.go.out View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/cmd/gofmt/gofmt_test.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/cmd/gofmt/rewrite.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/goinstall/download.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/cmd/govet/govet.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/goyacc/goyacc.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/hgpatch/main.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/asn1/common.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/bytes/bytes.go View 1 1 chunk +20 lines, -6 lines 0 comments Download
M src/pkg/bytes/bytes_test.go View 1 6 chunks +16 lines, -3 lines 0 comments Download
M src/pkg/compress/lzw/reader_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/crypto/x509/verify.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/pkg/debug/proc/proc_linux.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/exec/exec_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
src/pkg/exec/lp_plan9.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/exec/lp_unix.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/exec/lp_windows.go View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/exp/ogle/cmd.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/go/ast/print_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/go/build/dir.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/go/doc/comment.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/html/token_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/http/cgi/host.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/http/cgi/host_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
src/pkg/http/cookie.go View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/http/fs.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/http/request.go View 1 3 chunks +4 lines, -4 lines 0 comments Download
M src/pkg/http/response.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/http/server.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/http/spdy/read.go View 1 1 chunk +1 line, -1 line 0 comments Download
src/pkg/http/transfer.go View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/http/transport.go View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/http/url.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/pkg/mail/message.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/mime/mediatype.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/patch/patch.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/path/filepath/path.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/rpc/server.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/debug/stack.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/debug/stack_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/smtp/smtp.go View 1 2 chunks +3 lines, -3 lines 0 comments Download
M src/pkg/smtp/smtp_test.go View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/pkg/strconv/fp_test.go View 1 3 chunks +3 lines, -3 lines 0 comments Download
M src/pkg/strings/strings.go View 1 1 chunk +20 lines, -6 lines 0 comments Download
M src/pkg/strings/strings_test.go View 1 2 6 chunks +17 lines, -5 lines 0 comments Download
M src/pkg/template/execute.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/template/parse.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/testing/testing.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/unicode/maketables.go View 1 6 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 12
r
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 10 months ago (2011-06-27 03:18:22 UTC) #1
adg
comments on gofix http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go File src/cmd/gofix/stringssplit.go (right): http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go#newcode13 src/cmd/gofix/stringssplit.go:13: var _ = reflect.TypeOf why? http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go#newcode25 ...
12 years, 10 months ago (2011-06-27 03:28:50 UTC) #2
adg
LGTM
12 years, 10 months ago (2011-06-27 03:31:19 UTC) #3
dsymonds
LGTM http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go File src/cmd/gofix/stringssplit.go (right): http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go#newcode13 src/cmd/gofix/stringssplit.go:13: var _ = reflect.TypeOf dreg? http://codereview.appspot.com/4661051/diff/2001/src/pkg/strings/strings_test.go File src/pkg/strings/strings_test.go ...
12 years, 10 months ago (2011-06-27 03:33:34 UTC) #4
r2
it does bytes too, but the other names mention the package. i'd prefer to leave ...
12 years, 10 months ago (2011-06-27 03:48:02 UTC) #5
r
Hello golang-dev@googlegroups.com, adg@golang.org, dsymonds@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
12 years, 10 months ago (2011-06-27 03:48:35 UTC) #6
dsymonds
LGTM http://codereview.appspot.com/4661051/diff/4002/src/cmd/gofix/stringssplit.go File src/cmd/gofix/stringssplit.go (right): http://codereview.appspot.com/4661051/diff/4002/src/cmd/gofix/stringssplit.go#newcode67 src/cmd/gofix/stringssplit.go:67: // If not, ename and keep the argument ...
12 years, 10 months ago (2011-06-27 03:51:23 UTC) #7
brainman
LGTM
12 years, 10 months ago (2011-06-27 04:16:30 UTC) #8
rsc
LGTM
12 years, 10 months ago (2011-06-27 17:59:13 UTC) #9
r
*** Submitted as http://code.google.com/p/go/source/detail?r=a58983161b47 *** strings.Split: make the default to split all. Change the signature ...
12 years, 10 months ago (2011-06-27 23:43:22 UTC) #10
tanujbaware
On 2011/06/27 03:33:34, dsymonds wrote: > LGTM > > http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go > File src/cmd/gofix/stringssplit.go (right): > ...
1 year, 3 months ago (2023-01-10 14:44:14 UTC) #11
tanujbaware
1 year, 3 months ago (2023-01-10 14:44:53 UTC) #12
Message was sent while issue was closed.
On 2023/01/10 14:44:14, tanujbaware wrote:
> On 2011/06/27 03:33:34, dsymonds wrote:
> > LGTM
> > 
> >
http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go
> > File src/cmd/gofix/stringssplit.go (right):
> > 
> >
>
http://codereview.appspot.com/4661051/diff/2001/src/cmd/gofix/stringssplit.go...
> > src/cmd/gofix/stringssplit.go:13: var _ = reflect.TypeOf
> > dreg?
> > 
> >
>
http://codereview.appspot.com/4661051/diff/2001/src/pkg/strings/strings_test.go
> > File src/pkg/strings/strings_test.go (right):
> > 
> >
>
http://codereview.appspot.com/4661051/diff/2001/src/pkg/strings/strings_test....
> > src/pkg/strings/strings_test.go:241: t.Errorf("Split disagrees
withSplitN(%q,
> > %q, %d) = %v; want %v", tt.s, tt.sep, tt.n, b, a)
> > "withSplitN": missing space
> > 
> >
>
http://codereview.appspot.com/4661051/diff/2001/src/pkg/strings/strings_test....
> > src/pkg/strings/strings_test.go:277: t.Errorf("SplitAfter disagrees
> > withSplitAfterN(%q, %q, %d) = %v; want %v", tt.s, tt.sep, tt.n, b, a)
> > ditto
> > javascript://alert(1);
> https://google.com
Sign in to reply to this message.

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