code review 2305043: fmt: allow %d on []byte

fmt: allow %d on []byte
Fixes issue 1159.

[r, 2010-10-04 03:22:57]

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

I'd like you to review this change.

[adg, 2010-10-04 09:27:21]

LGTM

On 4 October 2010 14:22,  <r@golang.org> wrote:
> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> fmt: allow %d on []byte
> Fixes issue 1159.
>
> Please review this at http://codereview.appspot.com/2305043/
>
> Affected files:
>  M src/pkg/fmt/fmt_test.go
>  M src/pkg/fmt/print.go
>
>
> Index: src/pkg/fmt/fmt_test.go
> ===================================================================
> --- a/src/pkg/fmt/fmt_test.go
> +++ b/src/pkg/fmt/fmt_test.go
> @@ -317,6 +317,8 @@
>        // slices with other formats
>        fmtTest{"%#x", []int{1, 2, 15}, `[0x1 0x2 0xf]`},
>        fmtTest{"%x", []int{1, 2, 15}, `[1 2 f]`},
> +       fmtTest{"%d", []int{1, 2, 15}, `[1 2 15]`},
> +       fmtTest{"%d", []byte{1, 2, 15}, `[1 2 15]`},
>        fmtTest{"%q", []string{"a", "b"}, `["a" "b"]`},
>
>        // renamings
> @@ -334,6 +336,7 @@
>        fmtTest{"%X", renamedUint64(17), "11"},
>        fmtTest{"%o", renamedUintptr(18), "22"},
>        fmtTest{"%x", renamedString("thing"), "7468696e67"},
> +       fmtTest{"%d", renamedBytes([]byte{1, 2, 15}), `[1 2 15]`},
>        fmtTest{"%q", renamedBytes([]byte("hello")), `"hello"`},
>        fmtTest{"%v", renamedFloat(11), "11"},
>        fmtTest{"%v", renamedFloat32(22), "22"},
> Index: src/pkg/fmt/print.go
> ===================================================================
> --- a/src/pkg/fmt/print.go
> +++ b/src/pkg/fmt/print.go
> @@ -439,7 +439,7 @@
>  }
>
>  func (p *pp) fmtBytes(v []byte, verb int, goSyntax bool, depth int, value
> interface{}) {
> -       if verb == 'v' {
> +       if verb == 'v' || verb == 'd' {
>                if goSyntax {
>                        p.buf.Write(bytesBytes)
>                } else {
>
>
>

[r, 2010-10-04 09:57:52]

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

fmt: allow %d on []byte
Fixes issue 1159.

R=rsc, adg
CC=golang-dev
http://codereview.appspot.com/2305043

[rog, 2010-10-04 10:43:31]

this is interesting.

it seem slightly inconsistent, and the documentation has nothing
to say about behaviour with respect to slices other
than slices of byte, AFAICS.

fmt.Sprintf("%d", []int{10, 11, 12}) -> "[10 11 12]"
fmt.Sprintf("%x", []int{10, 11, 12}) -> "[a b c]"
fmt.Sprintf("%d", []byte{10, 11, 12}) -> "[10 11 12]"
fmt.Sprintf("%x", []byte{10, 11, 12}) -> "0a0b0c"

at the least, i think it should be documented that %d (and %o and... ?)
work on slices; and if Printf works, why not Scanf too?

i had previously assumed that for anything other than the basic
types, it was necessary to use %v. but

    fmt.Printf("%x\n", struct{a, b int; c string}{10,11,"foo"})

works too. what other interesting behaviour is lurking there?

On 4 October 2010 10:57,  <r@golang.org> wrote:
> *** Submitted as
> http://code.google.com/p/go/source/detail?r=70ddef42f94e ***
>
> fmt: allow %d on []byte
> Fixes issue 1159.
>
> R=rsc, adg
> CC=golang-dev
> http://codereview.appspot.com/2305043
>
>
> http://codereview.appspot.com/2305043/
>

[r2, 2010-10-04 11:56:25]

you're right that things aren't as well documented as they could be.  (is printf
ever well documented?)

but here's how to think about it.  after a recent rewrite, the verbs are
secondary.  that is, printf prints according to the type of the argument first. 
the verb can color the result, as you see with the struct example.  for
composite objects, the effect is mostly to apply the verb to its elements.

there's probably a good way to explain this, and i'm sure there are cases one
can construct where you don't get what you expect.  %d on []byte was one; %o
etc. on []byte won't work at the moment, but that's a lack of the implementation
rather than the approach.

-rob

On Oct 4, 2010, at 12:43 PM, roger peppe wrote:

> this is interesting.
> 
> it seem slightly inconsistent, and the documentation has nothing
> to say about behaviour with respect to slices other
> than slices of byte, AFAICS.
> 
> fmt.Sprintf("%d", []int{10, 11, 12}) -> "[10 11 12]"
> fmt.Sprintf("%x", []int{10, 11, 12}) -> "[a b c]"
> fmt.Sprintf("%d", []byte{10, 11, 12}) -> "[10 11 12]"
> fmt.Sprintf("%x", []byte{10, 11, 12}) -> "0a0b0c"
> 
> at the least, i think it should be documented that %d (and %o and... ?)
> work on slices; and if Printf works, why not Scanf too?
> 
> i had previously assumed that for anything other than the basic
> types, it was necessary to use %v. but
> 
>    fmt.Printf("%x\n", struct{a, b int; c string}{10,11,"foo"})
> 
> works too. what other interesting behaviour is lurking there?
> 
> On 4 October 2010 10:57,  <r@golang.org> wrote:
>> *** Submitted as
>> http://code.google.com/p/go/source/detail?r=70ddef42f94e ***
>> 
>> fmt: allow %d on []byte
>> Fixes issue 1159.
>> 
>> R=rsc, adg
>> CC=golang-dev
>> http://codereview.appspot.com/2305043
>> 
>> 
>> http://codereview.appspot.com/2305043/
>>