code review 4140041: bytes: Fix bugs in buffer.ReadBytes

bytes: Fix bugs in buffer.ReadBytes

[eds, 2011-02-08 04:33:21]

Hello golang-dev@googlegroups.com,

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

[mattn, 2011-02-08 04:49:42]

LGTM

On 2011/02/08 04:33:21, eds wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://go.googlecode.com/hg/

[r, 2011-02-08 19:08:39]

http://codereview.appspot.com/4140041/diff/5001/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

http://codereview.appspot.com/4140041/diff/5001/src/pkg/bytes/buffer_test.go#...
src/pkg/bytes/buffer_test.go:359: {[]byte{}, 0, [][]byte{[]byte{}}, os.EOF},
these tests would be a lot easier to read if buffer and expected were []string
and you did simple conversions in the test function.

http://codereview.appspot.com/4140041/diff/5001/src/pkg/bytes/buffer_test.go#...
src/pkg/bytes/buffer_test.go:364: {[]byte("alpha beta gamma"), ' ',
[][]byte{[]byte("alpha ")}, nil},
do all three words, use newlines to separate, and you should see err == nil
still.
then do another case without a terminal newline, should see same words but EOF.

http://codereview.appspot.com/4140041/diff/5001/src/pkg/bytes/buffer_test.go#...
src/pkg/bytes/buffer_test.go:371: bytes, err := buf.ReadBytes(test.delim)
this test isn't set up right.
you want to have a sequence of reads all getting nil until the last, which
should see test.err.

[eds, 2011-02-09 04:38:56]

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

Please take another look.

[r, 2011-02-09 23:15:42]

test looks much better. one small simplification.

http://codereview.appspot.com/4140041/diff/17001/src/pkg/bytes/buffer_test.go
File src/pkg/bytes/buffer_test.go (right):

http://codereview.appspot.com/4140041/diff/17001/src/pkg/bytes/buffer_test.go...
src/pkg/bytes/buffer_test.go:374: if !Equal(bytes, []byte(expected)) {
if string(bytes) != expected

[eds, 2011-02-10 08:46:33]

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

Please take another look.

[r, 2011-02-10 19:43:14]

LGTM

[r, 2011-02-10 19:43:58]

package has changed. please update and re-mail.

[eds, 2011-02-11 04:27:22]

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

Please take another look.

[rsc, 2011-02-11 17:08:29]

add

Fixes issue 1498.

[rsc, 2011-02-11 17:39:22]

*** Submitted as c4e5db759744 ***

bytes: fix bugs in buffer.ReadBytes

Fixes issue 1498.

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

Committer: Russ Cox <rsc@golang.org>