Re: code review 4709044: json: add omitzero struct tag option (issue4709044)

[brad...@golang.org]

Hello r...@golang.org, dsym...@golang.org, r...@google.com (cc:
golan...@googlegroups.com),

Please take another look.

http://codereview.appspot.com/4709044/

[brad...@golang.org]

[David Symonds]

I'm personally much happier with that.

[r...@golang.org]

looks pretty good

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go
File src/pkg/json/encode.go (right):

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go#newcode41
src/pkg/json/encode.go:41: // becomes a member of the object unless the
field's "omitzero" option
Can we call this omitempty?
It's not really zero (you don't check for zero structs for example).
Let's use the same definition exp/template just committed
(godoc exp/template |grep -C 5 empty, look in code for isTruth)

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go#newcode275
src/pkg/json/encode.go:275: tag, omitZero := "", false
s/""/f.Name/

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go#newcode278
src/pkg/json/encode.go:278: tag = ss[0]
if isValidTag(ss[0]) { tag = ss[0] }

might as well put the logic with the parser

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go#newcode294
src/pkg/json/encode.go:294: if tag != "" && isValidTag(tag) {
e.string(tag)

http://codereview.appspot.com/4709044/

[Brad Fitzpatrick]

omitempty makes it sounds like it's only for strings and lists.

b := 0  // empty?

But if that's the wording we use elsewhere, I won't fight it.

[r...@golang.org]

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go
File src/pkg/json/encode.go (right):

http://codereview.appspot.com/4709044/diff/11002/src/pkg/json/encode.go#newcode205
src/pkg/json/encode.go:205: return false
maybe return true?

http://codereview.appspot.com/4709044/

[brad...@golang.org]

Hello r...@golang.org, dsym...@golang.org, r...@google.com, r...@golang.org

[r...@golang.org]

LGTM

http://codereview.appspot.com/4709044/diff/14001/src/pkg/json/encode.go
File src/pkg/json/encode.go (right):

http://codereview.appspot.com/4709044/diff/14001/src/pkg/json/encode.go#newcode42
src/pkg/json/encode.go:42: // specified and it has an empty or zero
value. The object's default key
unless the field is empty and its tag specifies the "omitempty" option.
The empty values are false, 0, any nil pointer or interface value,
and any array, slice, map, or string of length zero.

http://codereview.appspot.com/4709044/

[brad...@golang.org]

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

json: add omitempty struct tag option

Fixes issue 2032

R=rsc, dsymonds, r, r
CC=golang-dev
http://codereview.appspot.com/4709044

http://codereview.appspot.com/4709044/