code review 5624047: go/scanner: clean up error interface

go/scanner: clean up error interface

Issue 2856 asks for a rename of a few methods to a
more idiomatic Go style. This is a very early API
that evolved organically throughout the years.
Together with the fact that ErrorVectors were embedded
in other data structures (e.g. Parser), just renaming
methods (e.g. GetError -> Error) would lead to undesired
behavior (e.g., Parser would act like an Error). Instead,
cleaned up API a bit more:

- removed ErrorVector in favor of ErrorList (already
present)
- simplified Scanner.Init by making the error handler a
function instead of requiring an ErrorHandler implementation
- adjusted helper functions accordingly
- updated Go 1 doc

Fixes issue 2856.

[gri, 2012-02-03 23:54:13]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2012-02-06 17:38:17]

http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl
File doc/go1.tmpl (right):

http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl#newcode865
doc/go1.tmpl:865: an <code>ErrorVector</code> in a client of the scanner, now a
client should simply maintain
s/simply //

http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go
File src/pkg/go/scanner/errors.go (right):

http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go#...
src/pkg/go/scanner/errors.go:105: // OrNil returns nil for an empty ErrorList,
otherwise it returns the list.
We have written this method in a few other places, and always as Err() error.
OrNil focuses too much on a subtle detail of the implementation.

// Err returns an error equivalent to this error list.
// If the list is empty, Err returns nil.
func (p ErrorList) Err() error {

[gri, 2012-02-06 17:43:37]

PTAL

http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl
File doc/go1.tmpl (right):

http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl#newcode865
doc/go1.tmpl:865: an <code>ErrorVector</code> in a client of the scanner, now a
client should simply maintain
On 2012/02/06 17:38:18, rsc wrote:
> s/simply //

Done.

http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go
File src/pkg/go/scanner/errors.go (right):

http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go#...
src/pkg/go/scanner/errors.go:105: // OrNil returns nil for an empty ErrorList,
otherwise it returns the list.
On 2012/02/06 17:38:18, rsc wrote:
> We have written this method in a few other places, and always as Err() error.
> OrNil focuses too much on a subtle detail of the implementation.
> 
> // Err returns an error equivalent to this error list.
> // If the list is empty, Err returns nil.
> func (p ErrorList) Err() error {

Done.

[gri, 2012-02-06 17:45:42]

One sec. This needs some other adjustments.
- gri

On Mon, Feb 6, 2012 at 9:43 AM,  <gri@golang.org> wrote:
> PTAL
>
>
>
> http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl
> File doc/go1.tmpl (right):
>
> http://codereview.appspot.com/5624047/diff/1003/doc/go1.tmpl#newcode865
> doc/go1.tmpl:865: an <code>ErrorVector</code> in a client of the
> scanner, now a client should simply maintain
> On 2012/02/06 17:38:18, rsc wrote:
>>
>> s/simply //
>
>
> Done.
>
>
> http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go
> File src/pkg/go/scanner/errors.go (right):
>
>
http://codereview.appspot.com/5624047/diff/1003/src/pkg/go/scanner/errors.go#...
> src/pkg/go/scanner/errors.go:105: // OrNil returns nil for an empty
> ErrorList, otherwise it returns the list.
> On 2012/02/06 17:38:18, rsc wrote:
>>
>> We have written this method in a few other places, and always as Err()
>
> error.
>>
>> OrNil focuses too much on a subtle detail of the implementation.
>
>
>> // Err returns an error equivalent to this error list.
>> // If the list is empty, Err returns nil.
>> func (p ErrorList) Err() error {
>
>
> Done.
>
> http://codereview.appspot.com/5624047/

[gri, 2012-02-06 18:00:57]

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

Please take another look.

[rsc, 2012-02-08 19:30:46]

LGTM
Sorry about the delay.

http://codereview.appspot.com/5624047/diff/14001/doc/go1.tmpl
File doc/go1.tmpl (right):

http://codereview.appspot.com/5624047/diff/14001/doc/go1.tmpl#newcode954
doc/go1.tmpl:954: an <code>ErrorList</code> instead.
s/ instead//
Already in this sentence once.

[gri, 2012-02-08 19:41:34]

*** Submitted as 531a26e7c3bf ***

go/scanner: clean up error interface

Issue 2856 asks for a rename of a few methods to a
more idiomatic Go style. This is a very early API
that evolved organically throughout the years.
Together with the fact that ErrorVectors were embedded
in other data structures (e.g. Parser), just renaming
methods (e.g. GetError -> Error) would lead to undesired
behavior (e.g., Parser would act like an Error). Instead,
cleaned up API a bit more:

- removed ErrorVector in favor of ErrorList (already
present)
- simplified Scanner.Init by making the error handler a
function instead of requiring an ErrorHandler implementation
- adjusted helper functions accordingly
- updated Go 1 doc

Fixes issue 2856.

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