code review 699041: xml: Added line numbers to syntax errors.

xml: Added line numbers to syntax errors.

[Kyle C, 2010-03-23 19:57:38]

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

I'd like you to review this change.

[rsc1, 2010-03-23 20:04:00]

Looks pretty good, thanks.

http://codereview.appspot.com/699041/diff/2001/3001
File src/pkg/xml/xml.go (right):

http://codereview.appspot.com/699041/diff/2001/3001#newcode29
src/pkg/xml/xml.go:29: msg  string
Make these exported (upper case) so clients can inspect Msg and Line themselves.

http://codereview.appspot.com/699041/diff/2001/3001#newcode33
src/pkg/xml/xml.go:33: func (e SyntaxError) String() string {
make this e *SyntaxError

and return &SyntaxError{...} from the method below.

The convention is that error structures are always pointers
when stored in interfaces.

http://codereview.appspot.com/699041/diff/2001/3002
File src/pkg/xml/xml_test.go (right):

http://codereview.appspot.com/699041/diff/2001/3002#newcode359
src/pkg/xml/xml_test.go:359: testInput := `<P>Foo<P>
probably clearer to use a normal string

testInput := "<P>Foo<P>\n\n<P>Bar</>"

[Kyle C, 2010-03-23 20:11:10]

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

Please take another look.

[Kyle C, 2010-03-23 20:11:14]

http://codereview.appspot.com/699041/diff/2001/3001
File src/pkg/xml/xml.go (right):

http://codereview.appspot.com/699041/diff/2001/3001#newcode29
src/pkg/xml/xml.go:29: msg  string
On 2010/03/23 20:04:00, rsc1 wrote:
> Make these exported (upper case) so clients can inspect Msg and Line
themselves.
> 

Done.

http://codereview.appspot.com/699041/diff/2001/3001#newcode33
src/pkg/xml/xml.go:33: func (e SyntaxError) String() string {
On 2010/03/23 20:04:00, rsc1 wrote:
> make this e *SyntaxError
> 
> and return &SyntaxError{...} from the method below.
> 
> The convention is that error structures are always pointers
> when stored in interfaces.
> 

Done.

http://codereview.appspot.com/699041/diff/2001/3002
File src/pkg/xml/xml_test.go (right):

http://codereview.appspot.com/699041/diff/2001/3002#newcode359
src/pkg/xml/xml_test.go:359: testInput := `<P>Foo<P>
On 2010/03/23 20:04:00, rsc1 wrote:
> probably clearer to use a normal string
> 
> testInput := "<P>Foo<P>\n\n<P>Bar</>"
> 

Done.

[rsc1, 2010-03-23 20:17:47]

LGTM

thanks

[rsc, 2010-03-23 20:20:19]

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

xml: add line numbers to syntax errors.

R=rsc
CC=golang-dev
http://codereview.appspot.com/699041

Committer: Russ Cox <rsc@golang.org>