code review 5331045: fmt: handle os.Error values

fmt: handle os.Error values

Handling os.Error is no different than handling fmt.Stringer
here, so the code is redundant now, but it will be necessary
once error goes in.

Adding it now will make gofix fix it.

[rsc, 2011-10-28 01:42:56]

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

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

[r, 2011-10-28 03:16:28]

http://codereview.appspot.com/5331045/diff/3/src/pkg/fmt/print.go
File src/pkg/fmt/print.go (right):

http://codereview.appspot.com/5331045/diff/3/src/pkg/fmt/print.go#newcode646
src/pkg/fmt/print.go:646: p.printField(err.String(), verb, plus, false, depth)
the duplication is avoidable. schematically,

if ... { str = String() }
else if ... { str = Error() }
if ok {
}

or even a type switch instead of two ifs

another question is what the right order is. i might put error first. the
question is: do we expect errors and strings to print differently if they're
both in the same type? if so, error is more specific and so should go first,
maybe. but it's arguably moot.

regardless it might be worth documenting the order.

[rsc, 2011-10-28 03:24:53]

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

Please take another look.

[rsc, 2011-10-28 03:26:06]

I added doc.go to this CL and swapped the order.

The duplication is less avoidable than it looks.
All that stuff before the .String() or .Error()
has to happen before the call: it sets up the
correct behavior if the call panics.

Russ

[r, 2011-10-28 04:07:24]

LGTM
but how about a type switch?

http://codereview.appspot.com/5331045/diff/9001/src/pkg/fmt/doc.go
File src/pkg/fmt/doc.go (right):

http://codereview.appspot.com/5331045/diff/9001/src/pkg/fmt/doc.go#newcode92
src/pkg/fmt/doc.go:92: If an operand implements the error interface, the Error
method
s/If/Next, if/

http://codereview.appspot.com/5331045/diff/9001/src/pkg/fmt/doc.go#newcode96
src/pkg/fmt/doc.go:96: If an operand implements method String() string that
method
s/If/Finally, if/

[rsc, 2011-10-28 04:13:31]

On Thu, Oct 27, 2011 at 21:07,  <r@golang.org> wrote:
> LGTM
> but how about a type switch?

Done.

[rsc, 2011-10-28 04:20:49]

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

fmt: handle os.Error values

Handling os.Error is no different than handling fmt.Stringer
here, so the code is redundant now, but it will be necessary
once error goes in.

Adding it now will make gofix fix it.

R=r
CC=golang-dev
http://codereview.appspot.com/5331045