code review 57140043: encoding/json: mention escaping of '&'

encoding/json: mention escaping of '&'
Fixes issue 7034.

[minux1, 2014-01-27 01:47:24]

Hello golang-codereviews@googlegroups.com,

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

[iant, 2014-01-27 16:39:04]

https://codereview.appspot.com/57140043/diff/70001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/57140043/diff/70001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode.go:47: // Ampersand "&" is also escaped to "\u0026"
for consistency with Compact.
I don't think "consistency with Compact" is the reason.  Compact actually does
not escape '&'--in compact that is only done if escape is true, but Compact
passes escape as false.  Based on http://golang.org/issue/3127, we escape '&'
for the same reason we escape '<' and '>'.

[minux1, 2014-01-28 00:36:06]

On 2014/01/27 16:39:04, iant wrote:
>
https://codereview.appspot.com/57140043/diff/70001/src/pkg/encoding/json/enco...
> src/pkg/encoding/json/encode.go:47: // Ampersand "&" is also escaped to
"\u0026"
> for consistency with Compact.
> I don't think "consistency with Compact" is the reason.  Compact actually does
> not escape '&'--in compact that is only done if escape is true, but Compact
> passes escape as false.  Based on http://golang.org/issue/3127, we escape '&'
> for the same reason we escape '<' and '>'.
I get that information from https://codereview.appspot.com/12708044, perhaps i'm
misreading
the description there?

[iant, 2014-01-28 01:39:19]

On Mon, Jan 27, 2014 at 4:36 PM,  <minux.ma@gmail.com> wrote:
> On 2014/01/27 16:39:04, iant wrote:
>
>
https://codereview.appspot.com/57140043/diff/70001/src/pkg/encoding/json/enco...
>>
>> src/pkg/encoding/json/encode.go:47: // Ampersand "&" is also escaped
>
> to "\u0026"
>>
>> for consistency with Compact.
>> I don't think "consistency with Compact" is the reason.  Compact
>
> actually does
>>
>> not escape '&'--in compact that is only done if escape is true, but
>
> Compact
>>
>> passes escape as false.  Based on http://golang.org/issue/3127, we
>
> escape '&'
>>
>> for the same reason we escape '<' and '>'.
>
> I get that information from https://codereview.appspot.com/12708044,
> perhaps i'm misreading
> the description there?

The description there refers to the compactor, not to Compact.
Compact calls compact passing the escape parameter as false.  The code
in that CL is for code that calls compact passing the escape parameter
as true.  So I think the description for 12708044 is correct, provided
you don't confuse "the compactor" with "Compact".

Ian

[minux1, 2014-01-28 07:13:30]

PTAL.

On Mon, Jan 27, 2014 at 8:39 PM, Ian Lance Taylor <iant@golang.org> wrote:

> On Mon, Jan 27, 2014 at 4:36 PM,  <minux.ma@gmail.com> wrote:
> > I get that information from https://codereview.appspot.com/12708044,
> > perhaps i'm misreading
> > the description there?
>
> The description there refers to the compactor, not to Compact.
> Compact calls compact passing the escape parameter as false.  The code
> in that CL is for code that calls compact passing the escape parameter
> as true.  So I think the description for 12708044 is correct, provided
> you don't confuse "the compactor" with "Compact".
>
Indeed i was confused about the two. Thanks for the clarification.

[iant, 2014-01-28 14:30:54]

LGTM

Thanks.

[minux1, 2014-02-05 06:24:57]

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

encoding/json: mention escaping of '&'
Fixes issue 7034.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/57140043

[gobot, 2014-02-05 06:34:46]

This CL appears to have broken the windows-amd64 builder.

[minux1, 2014-02-05 06:36:46]

Not really.

On Wed, Feb 5, 2014 at 1:34 AM, <gobot@golang.org> wrote:

> This CL appears to have broken the windows-amd64 builder.
>
--- FAIL: TestGoroutineSwitch (0.50 seconds)
pprof_test.go:64: profile too short: [0x0 0x3 0x0 0x2710 0x0 0x0 0x1 0x0]
FAIL
FAIL runtime/pprof 10.168s