code review 129330043: fmt: print byte stringers correctly

fmt: print byte stringers correctly

type T byte
func (T) String() string { return "X" }

fmt.Sprintf("%s", []T{97, 98, 99, 100}) == "abcd"
fmt.Sprintf("%x", []T{97, 98, 99, 100}) == "61626364"
fmt.Sprintf("%v", []T{97, 98, 99, 100}) == "[X X X X]"

This change makes the last case print correctly.
Before, it would have been "[97 98 99 100]".

Fixes issue 8360.

[adg, 2014-08-18 07:08:26]

Hello golang-codereviews@googlegroups.com,

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

[kortschak, 2014-08-18 07:22:56]

Add test for fmt.Formatter case as well? This fixes that too (checked).

[adg, 2014-08-18 07:37:07]

R=r

[adg, 2014-08-18 07:49:30]

On 18 August 2014 17:22, <dan.kortschak@adelaide.edu.au> wrote:

> Add test for fmt.Formatter case as well? This fixes that too (checked).


I have done so, but now I'm not sure that it's correct.

Should a Formatter really be invoked for the %#v verb? The docs say yes,
but it doesn't seem right to me.

[r, 2014-08-18 15:58:41]

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go
File src/pkg/fmt/print.go (right):

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go#newc...
src/pkg/fmt/print.go:931: // - Handle []byte (or []uint8) with fmtBytes.
inside fmt, []byte and []uint8 are indistinguishable, so s/or/==/

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go#newc...
src/pkg/fmt/print.go:935: if typ := f.Type(); typ.Elem() == byteType ||
typ.Elem().Kind() == reflect.Uint8 && (verb == 's' || verb == 'q' || verb ==
'x') {
this is harder to understand than it needs to be, i think. leave the previous
version alone as a precursor since that's the defining condition for a byte
slice. like this (untested):

typ.Elem().Kind() == reflect.Uint8 && (typ.Elem() == byteType || verb == 's' ||
verb == 'q' || verb == 'x')

[adg, 2014-08-18 22:23:42]

PTAL

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go
File src/pkg/fmt/print.go (right):

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go#newc...
src/pkg/fmt/print.go:931: // - Handle []byte (or []uint8) with fmtBytes.
On 2014/08/18 15:58:40, r wrote:
> inside fmt, []byte and []uint8 are indistinguishable, so s/or/==/

Done.

https://codereview.appspot.com/129330043/diff/20001/src/pkg/fmt/print.go#newc...
src/pkg/fmt/print.go:935: if typ := f.Type(); typ.Elem() == byteType ||
typ.Elem().Kind() == reflect.Uint8 && (verb == 's' || verb == 'q' || verb ==
'x') {
On 2014/08/18 15:58:40, r wrote:
> this is harder to understand than it needs to be, i think. leave the previous
> version alone as a precursor since that's the defining condition for a byte
> slice. like this (untested):
> 
> typ.Elem().Kind() == reflect.Uint8 && (typ.Elem() == byteType || verb == 's'
||
> verb == 'q' || verb == 'x')
> 

Done.

[r, 2014-08-18 22:47:54]

LGTM

[adg, 2014-08-18 22:53:03]

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

fmt: print byte stringers correctly

type T byte
func (T) String() string { return "X" }

fmt.Sprintf("%s", []T{97, 98, 99, 100}) == "abcd"
fmt.Sprintf("%x", []T{97, 98, 99, 100}) == "61626364"
fmt.Sprintf("%v", []T{97, 98, 99, 100}) == "[X X X X]"

This change makes the last case print correctly.
Before, it would have been "[97 98 99 100]".

Fixes issue 8360.

LGTM=r
R=r, dan.kortschak
CC=golang-codereviews
https://codereview.appspot.com/129330043

[gobot, 2014-08-18 23:55:10]

This CL appears to have broken the plan9-386-cnielsen builder.
See http://build.golang.org/log/265b8631bbc47bb23684dc93331d0386c808a0ff