encoding/line: add

encoding/line: add

I needed a way to read lines without worrying about \n and \r\n.

[agl1, 2010-11-03 15:25:58]

This is a continuation of http://codereview.appspot.com/2545041, but I couldn't
persuade hg to believe that.

[r, 2010-11-03 18:18:44]

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go
File src/pkg/encoding/line/line.go (right):

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:5: // This package implements a Reader and Writer
which break and unbreak data at a
s/which/that/

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:6: // fixed line length, as needed by PEM
encoding.
s/as/such as is/

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:35: // first byte.
s/.$/, assuming correct UTF-8 follows./

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:35: // first byte.
why don't you use package utf8?

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:45: return 4
this could be caused by bad user data, in which case 4 is not the right answer.
if you used the utf8 package, it would handle this sort of problem for you.  if
not, you should at least behave the same (errors consume one byte and emit
FFFD), although that can't be done with this function alone.

p.s. the "partial rune problem" is a bitch, i know, but since you're breaking on
lines it's always ok to read up to a newline and avoid the issue.

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:112: // ReadLine tries to return a single line,
not including the end-of-line bytes.
it's not clear from the comment whether ReadLine reformats its input. that is,
does 'a single line' refer to the input or the returned text if the line is
short?

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line_te...
File src/pkg/encoding/line/line_test.go (right):

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line_te...
src/pkg/encoding/line/line_test.go:29: var testUnicodeOutput =
[]byte("012\n♣Ø⊗\n⊇∑±\n¥")
i bid 4 no trump

[agl1, 2010-11-04 14:25:51]

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go
File src/pkg/encoding/line/line.go (right):

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:5: // This package implements a Reader and Writer
which break and unbreak data at a
On 2010/11/03 18:18:44, r wrote:
> s/which/that/

Done.

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:6: // fixed line length, as needed by PEM
encoding.
On 2010/11/03 18:18:44, r wrote:
> s/as/such as is/

Done.

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:45: return 4
On 2010/11/03 18:18:44, r wrote:
> this could be caused by bad user data, in which case 4 is not the right
answer.
> if you used the utf8 package, it would handle this sort of problem for you.

I did look at the utf8 package for a while, but I didn't see anything that would
work here. I could use FullRune as a test and then get the codepoint with
DecodeRune. But if FullRune returned false, I would have to buffer the partial
data. (Fundamentally, one would always have to buffer a partial rune in order to
catch invalid UTF-8, right?)

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:112: // ReadLine tries to return a single line,
not including the end-of-line bytes.
On 2010/11/03 18:18:44, r wrote:
> it's not clear from the comment whether ReadLine reformats its input. that is,
> does 'a single line' refer to the input or the returned text if the line is
> short?

Sorry, I don't understand this comment. If I write "ReadLine tries to return a
single line from the input.", does that clear it up?

[rsc1, 2010-11-04 14:51:39]

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go
File src/pkg/encoding/line/line.go (right):

http://codereview.appspot.com/2859041/diff/2001/src/pkg/encoding/line/line.go...
src/pkg/encoding/line/line.go:112: // ReadLine tries to return a single line,
not including the end-of-line bytes.
On 2010/11/04 14:25:51, agl1 wrote:
> On 2010/11/03 18:18:44, r wrote:
> > it's not clear from the comment whether ReadLine reformats its input. that
is,
> > does 'a single line' refer to the input or the returned text if the line is
> > short?
> 
> Sorry, I don't understand this comment. If I write "ReadLine tries to return a
> single line from the input.", does that clear it up?

ReadLine tries to return the next input line, not including the end-of-line
bytes.

[rsc, 2010-12-07 17:57:28]

Still planning to add this?

[agl1, 2010-12-21 15:00:50]

PTAL.

I've removed the line breaker since we decided that it was too specific for a
generic package. However, I still have several uses for the line reader so I
think it's still worthwhile.

[rsc, 2011-01-04 15:16:18]

LGTM

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line.go
File src/pkg/encoding/line/line.go (right):

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line....
src/pkg/encoding/line/line.go:41: l.buf = l.buf[:len(l.buf)-l.consumed]
This is fine.  Might be clearer as

n := copy(l.buf, l.buf[l.consumed:])
l.buf = l.buf[:n]

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line_...
File src/pkg/encoding/line/line_test.go (right):

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line_...
src/pkg/encoding/line/line_test.go:55: } else {
Errorf doesn't stop the test; do you really want } else { here?
(Fatalf does)

If you do switch to Fatalf, can still drop the else.

[agl1, 2011-01-08 15:31:08]

Submitted.

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line.go
File src/pkg/encoding/line/line.go (right):

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line....
src/pkg/encoding/line/line.go:41: l.buf = l.buf[:len(l.buf)-l.consumed]
On 2011/01/04 15:16:18, rsc wrote:
> This is fine.  Might be clearer as
> 
> n := copy(l.buf, l.buf[l.consumed:])
> l.buf = l.buf[:n]

Done.

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line_...
File src/pkg/encoding/line/line_test.go (right):

https://codereview.appspot.com/2859041/diff/15001/src/pkg/encoding/line/line_...
src/pkg/encoding/line/line_test.go:55: } else {
On 2011/01/04 15:16:18, rsc wrote:
> Errorf doesn't stop the test; do you really want } else { here?
> (Fatalf does)
> 
> If you do switch to Fatalf, can still drop the else.

Done.