code review 12703043: encoding/json: support encoding.TextMarshaler, encoding...

encoding/json: support encoding.TextMarshaler, encoding.TextUnmarshaler

[rsc, 2013-08-09 19:09:44]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-08-09 20:04:16]

https://codereview.appspot.com/12703043/diff/6001/src/pkg/encoding/json/encod...
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/12703043/diff/6001/src/pkg/encoding/json/encod...
src/pkg/encoding/json/encode.go:847: func (e *encodeState) stringBytes(s []byte)
(int, error) {
this is sad, that we have two giant copies of a non-trivial function that don't
differ at all.

and that you just fixed a bug in both of them.

at least they're adjacent, but could you add a warning comment above both of
them to keep the in sync?  or better: add a test that they're always equivalent
for a large set of test cases?

[rsc, 2013-08-14 04:32:27]

PTAL

I added a test for the two functions being in sync.

[bradfitz, 2013-08-14 18:04:53]

LGTM

https://codereview.appspot.com/12703043/diff/10001/src/pkg/encoding/json/deco...
File src/pkg/encoding/json/decode_test.go (right):

https://codereview.appspot.com/12703043/diff/10001/src/pkg/encoding/json/deco...
src/pkg/encoding/json/decode_test.go:79: *u = unmarshalerText{true} // All we
need to see that UnmarshalJson is called.
s/Json/JSON/

[rsc, 2013-08-14 18:56:10]

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

encoding/json: support encoding.TextMarshaler, encoding.TextUnmarshaler

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/12703043