code review 218050: XML: Change behavior of boolean unmarshaling

Change behavior of boolean unmarshaling

[hoisie, 2010-02-22 17:37:40]

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

I'd like you to review this change.

[rsc, 2010-02-22 17:49:25]

This assumes that boolean unmarshalling is broken.
It's behaving exactly as the doc comment says:

// Unmarshal maps an XML element to a bool by setting the bool to true.

I did this so that you can use a bool to tell whether
an element is there at all.  Whether that's correct
behavior might be debated, but that makes this not
a bug fix but a package change.

Russ

[hoisie, 2010-02-22 17:58:32]

I think it's very common for schemas to include boolean elements as "true" or
"false" : http://www.w3.org/TR/xmlschema-2/#boolean


On 2010/02/22 17:49:25, rsc wrote:
> This assumes that boolean unmarshalling is broken.
> It's behaving exactly as the doc comment says:
> 
> // Unmarshal maps an XML element to a bool by setting the bool to true.
> 
> I did this so that you can use a bool to tell whether
> an element is there at all.  Whether that's correct
> behavior might be debated, but that makes this not
> a bug fix but a package change.
> 
> Russ
>

[r, 2010-02-22 20:52:37]

changing reviewer to rsc.

[rsc, 2010-02-22 22:32:22]

that's fine.
i don't mind changing it, as long as the docs change,
which you've done.

the url you pointed at says that true and 1 are both
valid booleans.  you should probably add "1".

[hoisie, 2010-02-22 23:00:00]

done, and updated the docs. 


On 2010/02/22 22:32:22, rsc wrote:
> that's fine.
> i don't mind changing it, as long as the docs change,
> which you've done.
> 
> the url you pointed at says that true and 1 are both
> valid booleans.  you should probably add "1".

[rsc, 2010-02-22 23:08:15]

thanks

http://codereview.appspot.com/218050/diff/1022/1023
File src/pkg/xml/read.go (right):

http://codereview.appspot.com/218050/diff/1022/1023#newcode109
src/pkg/xml/read.go:109: // string value is "true" or "1"; otherwise it is set
to false
// string value is "true" or "1", or false otherwise.

note the period at the end of the sentence

http://codereview.appspot.com/218050/diff/1022/1024
File src/pkg/xml/xml_test.go (right):

http://codereview.appspot.com/218050/diff/1022/1024#newcode5
src/pkg/xml/xml_test.go:5: package xml
add test of 1 == true, 0 == false

[hoisie, 2010-02-22 23:14:34]

Done

On 2010/02/22 23:08:15, rsc wrote:
> thanks
> 
> http://codereview.appspot.com/218050/diff/1022/1023
> File src/pkg/xml/read.go (right):
> 
> http://codereview.appspot.com/218050/diff/1022/1023#newcode109
> src/pkg/xml/read.go:109: // string value is "true" or "1"; otherwise it is set
> to false
> // string value is "true" or "1", or false otherwise.
> 
> note the period at the end of the sentence
> 
> http://codereview.appspot.com/218050/diff/1022/1024
> File src/pkg/xml/xml_test.go (right):
> 
> http://codereview.appspot.com/218050/diff/1022/1024#newcode5
> src/pkg/xml/xml_test.go:5: package xml
> add test of 1 == true, 0 == false

[rsc, 2010-02-22 23:21:16]

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

xml: treat bool as value in Unmarshal

R=rsc
CC=golang-dev
http://codereview.appspot.com/218050

Committer: Russ Cox <rsc@golang.org>

[darkhelmet_darkhelmetlive.com, 2013-08-18 06:02:21]

Sorry to drag things back from the dead, but I'm trying to unmarshal a node 
in the "original" way.

The node is "<b/>" and it's either there or not, so I wanted to use a 
boolean to denote its presence. Except now it needs the string true or 
false in there. There's no custom Unmarshaler support like in 
encoding/json, so I'm not sure the best way to accomplish this.

Thoughts?

- Daniel

On Monday, February 22, 2010 10:58:32 AM UTC-7, hoi...@gmail.com wrote:
>
> I think it's very common for schemas to include boolean elements as
> "true" or "false" : http://www.w3.org/TR/xmlschema-2/#boolean
>
>
> On 2010/02/22 17:49:25, rsc wrote:
> > This assumes that boolean unmarshalling is broken.
> > It's behaving exactly as the doc comment says:
>
> > // Unmarshal maps an XML element to a bool by setting the bool to
> true.
>
> > I did this so that you can use a bool to tell whether
> > an element is there at all.  Whether that's correct
> > behavior might be debated, but that makes this not
> > a bug fix but a package change.
>
> > Russ
>
>
> http://codereview.appspot.com/218050/show
>
>

[Dominik Honnef, 2013-08-18 15:48:51]

There's support for custom (un)marshaling in encoding/xml since around CL
12556043.

On 2013/08/18 06:02:21, darkhelmet_darkhelmetlive.com wrote:
> Sorry to drag things back from the dead, but I'm trying to unmarshal a node 
> in the "original" way.
> 
> The node is "<b/>" and it's either there or not, so I wanted to use a 
> boolean to denote its presence. Except now it needs the string true or 
> false in there. There's no custom Unmarshaler support like in 
> encoding/json, so I'm not sure the best way to accomplish this.
> 
> Thoughts?
> 
> - Daniel
> 
> On Monday, February 22, 2010 10:58:32 AM UTC-7, mailto:hoi...@gmail.com wrote:
> >
> > I think it's very common for schemas to include boolean elements as
> > "true" or "false" : http://www.w3.org/TR/xmlschema-2/#boolean
> >
> >
> > On 2010/02/22 17:49:25, rsc wrote:
> > > This assumes that boolean unmarshalling is broken.
> > > It's behaving exactly as the doc comment says:
> >
> > > // Unmarshal maps an XML element to a bool by setting the bool to
> > true.
> >
> > > I did this so that you can use a bool to tell whether
> > > an element is there at all.  Whether that's correct
> > > behavior might be debated, but that makes this not
> > > a bug fix but a package change.
> >
> > > Russ
> >
> >
> > http://codereview.appspot.com/218050/show
> >
> >

[rsc, 2013-09-03 20:28:29]

You can use a *struct{} and check whether it is nil after unmarshal.