[go] encoding/json: rename Indent method to SetIndent

[Russ Cox (Gerrit)]

Reviewers: Ian Lance Taylor

Russ Cox uploaded a change:
https://go-review.googlesource.com/23295

encoding/json: rename Indent method to SetIndent

CL 21057 added this method during the Go 1.7 cycle
(so it is not yet released and still possible to revise).

This makes it clearer that the method is not doing something
(like func Indent does), but just changing a setting about doing
something later.

Also document that this is in some sense irreversible.
I think that's probably a mistake but the original CL discussion
claimed it as a feature, so I'll leave it alone.

For #6492.

Change-Id: If4415c869a9196501056c143811a308822d5a420
---
M src/encoding/json/stream.go
M src/encoding/json/stream_test.go
2 files changed, 10 insertions(+), 4 deletions(-)



diff --git a/src/encoding/json/stream.go b/src/encoding/json/stream.go
index a362704..8686784 100644
--- a/src/encoding/json/stream.go
+++ b/src/encoding/json/stream.go
@@ -219,9 +219,15 @@
return err
}

-// Indent sets the encoder to format each encoded value with Indent.
-func (enc *Encoder) Indent(prefix, indent string) {
- enc.indentBuf = new(bytes.Buffer)
+// SetIndent instructs the encoder to format each subsequent encoded
+// value as if indented by the package-level function Indent(prefix,
indent).
+// It is not possible to subsequently disable indentation:
+// even Indent("", "") will still insert additional newline characters
before each
+// element in a JSON object or array.
+func (enc *Encoder) SetIndent(prefix, indent string) {
+ if enc.indentBuf == nil {
+ enc.indentBuf = new(bytes.Buffer)
+ }
enc.indentPrefix = prefix
enc.indentValue = indent
}
diff --git a/src/encoding/json/stream_test.go
b/src/encoding/json/stream_test.go
index 0d578ce..ed6a391 100644
--- a/src/encoding/json/stream_test.go
+++ b/src/encoding/json/stream_test.go
@@ -77,7 +77,7 @@
func TestEncoderIndent(t *testing.T) {
var buf bytes.Buffer
enc := NewEncoder(&buf)
- enc.Indent(">", ".")
+ enc.SetIndent(">", ".")
for _, v := range streamTest {
enc.Encode(v)
}

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=953cd316

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1: TryBot-Result+1

TryBots are happy.

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Caleb Spare (Gerrit)]

Caleb Spare has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1:

(2 comments)

https://go-review.googlesource.com/#/c/23295/1//COMMIT_MSG
Commit Message:

PS1, Line 17: I think that's probably a mistake but the original CL

discussion
: claimed it as a feature, so I'll leave it alone.

Well, the new documentation sure makes this sound like an API mistake as
well :)

I cannot find where in the original CL discussion this irreversibility was
claimed as a feature. I don't think it was discussed. FWIW when I
implemented this feature (and DisableHTMLEncoding) I was following the cue
of the existing UseNumber method.

I'd be fine with making SetIndent reversible; do you have an API
suggestion, though?


https://go-review.googlesource.com/#/c/23295/1/src/encoding/json/stream.go
File src/encoding/json/stream.go:

PS1, Line 223: Indent(prefix, indent)
Consider replacing this with just "Indent". (The package-level function
takes more arguments, so I'm not sure that listing prefix, indent adds
clarity.)


--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Ian Lance Taylor (Gerrit)]

Ian Lance Taylor has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1:

(2 comments)

https://go-review.googlesource.com/#/c/23295/1//COMMIT_MSG
Commit Message:

PS1, Line 17: I think that's probably a mistake but the original CL

discussion
: claimed it as a feature, so I'll leave it alone.

> Well, the new documentation sure makes this sound like an API mistake as
> we

In CL 21057 you suggested that Indent("", "") should add newlines. That
call seems like the natural way to reverse any previous SetIndent call.

https://go-review.googlesource.com/#/c/23295/1/src/encoding/json/stream.go
File src/encoding/json/stream.go:

PS1, Line 223: Indent(prefix, indent)
> Consider replacing this with just "Indent". (The package-level function
> tak

I think we need to list the arguments but we could say Indent(dst, src,
prefix, indent).

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Caleb Spare (Gerrit)]

Caleb Spare has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1:

(1 comment)

https://go-review.googlesource.com/#/c/23295/1//COMMIT_MSG
Commit Message:

PS1, Line 17: I think that's probably a mistake but the original CL

discussion
: claimed it as a feature, so I'll leave it alone.

> In CL 21057 you suggested that Indent("", "") should add newlines. That
> ca

Thanks Ian, I see what this is about now.

What I meant there is that the existing Indent function still inserts
newlines when given (prefix="", indent=""):
https://play.golang.org/p/pdgJUJFZEO

I thought it would be confusing if Encoder.SetIndent didn't allow for this
(admittedly weird) special case. Maybe we don't care, though?

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Ian Lance Taylor (Gerrit)]

Ian Lance Taylor has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1: Code-Review+2

(1 comment)

https://go-review.googlesource.com/#/c/23295/1//COMMIT_MSG
Commit Message:

PS1, Line 17: I think that's probably a mistake but the original CL

discussion
: claimed it as a feature, so I'll leave it alone.

> Thanks Ian, I see what this is about now.

I don't know enough about encoding/json to know whether we care or not.

Either way, it seems like this CL should go in when the comment is tweaked.

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 2:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=63230131

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>

Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 2:

Build is still in progress...
This change failed on linux-amd64-race:
See
https://storage.googleapis.com/go-build-log/63230131/linux-amd64-race_616ab9e1.log

Consult https://build.golang.org/ to see whether it's a new failure. Other
builds still in progress; subsequent failure notices suppressed until final
report.

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Andrew Gerrand (Gerrit)]

Andrew Gerrand has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 2: Code-Review+2

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 2: TryBot-Result-1

1 of 18 TryBots failed:
Failed on linux-amd64-race:
https://storage.googleapis.com/go-build-log/63230131/linux-amd64-race_616ab9e1.log

Consult https://build.golang.org/ to see whether they are new failures.

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 3:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=7a5137ed

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 3: TryBot-Result+1

TryBots are happy.

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Russ Cox (Gerrit)]

Russ Cox has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 1:

(3 comments)

https://go-review.googlesource.com/#/c/23295/1//COMMIT_MSG
Commit Message:

PS1, Line 17: I think that's probably a mistake but the original CL

discussion
: claimed it as a feature, so I'll leave it alone.

> I don't know enough about encoding/json to know whether we care or not.

OK, based on this discussion I have changed SetIndent("", "") to disable
indentation entirely, including newlines.

https://go-review.googlesource.com/#/c/23295/1/src/encoding/json/stream.go
File src/encoding/json/stream.go:

PS1, Line 223: Indent(prefix, indent)
> Consider replacing this with just "Indent". (The package-level function
> tak

Did what Ian suggested.

PS1, Line 223: Indent(prefix, indent)

> I think we need to list the arguments but we could say Indent(dst, src,
> pre

Done

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: Yes

[Russ Cox (Gerrit)]

Reviewers: Andrew Gerrand, Ian Lance Taylor, Gobot Gobot

Russ Cox uploaded a new patch set:

https://go-review.googlesource.com/23295

encoding/json: rename Indent method to SetIndent

CL 21057 added this method during the Go 1.7 cycle
(so it is not yet released and still possible to revise).

This makes it clearer that the method is not doing something
(like func Indent does), but just changing a setting about doing
something later.

Also document that this is in some sense irreversible.
I think that's probably a mistake but the original CL discussion
claimed it as a feature, so I'll leave it alone.

For #6492.

Change-Id: If4415c869a9196501056c143811a308822d5a420
---
M src/encoding/json/stream.go
M src/encoding/json/stream_test.go

2 files changed, 13 insertions(+), 6 deletions(-)

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

encoding/json: rename Indent method to SetIndent

Patch Set 4:

TryBots beginning. Status page: http://farmer.golang.org/try?commit=3ea6b0c7

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-HasComments: No

[Russ Cox (Gerrit)]

Russ Cox has submitted this change and it was merged.

encoding/json: rename Indent method to SetIndent

CL 21057 added this method during the Go 1.7 cycle
(so it is not yet released and still possible to revise).

This makes it clearer that the method is not doing something
(like func Indent does), but just changing a setting about doing
something later.

Also document that this is in some sense irreversible.
I think that's probably a mistake but the original CL discussion
claimed it as a feature, so I'll leave it alone.

For #6492.

Change-Id: If4415c869a9196501056c143811a308822d5a420

Reviewed-on: https://go-review.googlesource.com/23295
Reviewed-by: Ian Lance Taylor <ia...@golang.org>
Reviewed-by: Andrew Gerrand <a...@golang.org>
Run-TryBot: Russ Cox <r...@golang.org>

---
M src/encoding/json/stream.go
M src/encoding/json/stream_test.go

2 files changed, 13 insertions(+), 6 deletions(-)

Approvals:
Russ Cox: Run TryBots
Andrew Gerrand: Looks good to me, approved
Ian Lance Taylor: Looks good to me, approved

--
https://go-review.googlesource.com/23295
Gerrit-Reviewer: Andrew Gerrand <a...@golang.org>
Gerrit-Reviewer: Caleb Spare <ces...@gmail.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>