code review 4927043: bufio: handle a "\r\n" that straddles the buffer.

bufio: handle a "\r\n" that straddles the buffer.

Fixes issue 2164.

[adg, 2011-08-22 08:07:54]

Hello golang-dev@googlegroups.com,

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

[adg, 2011-08-22 08:22:01]

Fixed another small issue and added more test cases.

[r, 2011-08-23 05:14:54]

http://codereview.appspot.com/4927043/diff/5/src/pkg/bufio/bufio.go
File src/pkg/bufio/bufio.go (right):

http://codereview.appspot.com/4927043/diff/5/src/pkg/bufio/bufio.go#newcode302
src/pkg/bufio/bufio.go:302: if len(line) > 0 && line[len(line)-1] == '\r' {
can't this be made much simpler using Peek?

[adg, 2011-08-23 05:18:15]

On 23 August 2011 15:14,  <r@golang.org> wrote:
> can't this be made much simpler using Peek?

Can't. Peek calls fill which invalidates the line we're returning.

The function could be a hell of a lot simpler if we said buffers of
length 1 are forbidden.

[r2, 2011-08-23 05:25:23]

On Aug 23, 2011, at 3:18 PM, Andrew Gerrand wrote:

> On 23 August 2011 15:14,  <r@golang.org> wrote:
>> can't this be made much simpler using Peek?
> 
> Can't. Peek calls fill which invalidates the line we're returning.
> 
> The function could be a hell of a lot simpler if we said buffers of
> length 1 are forbidden.

SGTM

[adg, 2011-08-24 06:06:45]

Okay, I've uploaded a version that forbids 1-byte buffers.

On 23 August 2011 15:25, Rob 'Commander' Pike <r@google.com> wrote:
>
> On Aug 23, 2011, at 3:18 PM, Andrew Gerrand wrote:
>
>> On 23 August 2011 15:14,  <r@golang.org> wrote:
>>> can't this be made much simpler using Peek?
>>
>> Can't. Peek calls fill which invalidates the line we're returning.
>>
>> The function could be a hell of a lot simpler if we said buffers of
>> length 1 are forbidden.
>
> SGTM
>

[r, 2011-08-24 06:18:21]

LGTM

http://codereview.appspot.com/4927043/diff/11001/src/pkg/bufio/bufio.go
File src/pkg/bufio/bufio.go (right):

http://codereview.appspot.com/4927043/diff/11001/src/pkg/bufio/bufio.go#newco...
src/pkg/bufio/bufio.go:325: }
a lot of reslicing going on. it's fine as is, but how about
if line[len(line)-1]  == '\n' {
   drop := 1
   if line[len(line)-2] == '\r' {
      drop = 2
   }
   line = line[len(line)-drop]
}

[rsc, 2011-08-24 15:11:59]

LGTM

http://codereview.appspot.com/4927043/diff/5/src/pkg/bufio/bufio.go
File src/pkg/bufio/bufio.go (right):

http://codereview.appspot.com/4927043/diff/5/src/pkg/bufio/bufio.go#newcode327
src/pkg/bufio/bufio.go:327: panic("tried to rewind past start of buffer")
Should say bufio.

[adg, 2011-08-24 22:44:18]

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

bufio: handle a "\r\n" that straddles the buffer.

Fixes issue 2164.

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