Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: Indent is documented to "have no trailing newline", yet it sometimes does. #13520

Closed
dmitshur opened this issue Dec 7, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@dmitshur
Copy link
Member

commented Dec 7, 2015

Tested with go version go1.5.2 darwin/amd64.

I'm reading the documentation for json.Indent:

https://godoc.org/encoding/json#Indent

Specifically, this part:

The data appended to dst does not begin with the prefix nor any indentation, and has no trailing newline, to make it easier to embed inside other formatted JSON data.

But that doesn't always seem to be the case. Depending on whether the input has a trailing newline, so does the output. Here's a quick reproduce program:

https://play.golang.org/p/bSoN4Tl2q_

If I understood the documentation and the output correctly, does this mean there's a bug in json.Indent?

Edit: Sorry, I accidentally used the word "encode" a few times instead of "indent". That was a typo, now fixed.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 7, 2015

It's more likely we'll update the docs at this point. But leaving to @rsc to decide.

@bradfitz bradfitz added this to the Go1.6 milestone Dec 7, 2015

@dmitshur

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2015

I see.

For what it's worth, I'll comment that I found it surprising that a func json.Indent which is supposed to take poorly formatted/unformatted JSON input, would seemingly randomly produce slightly different output (before I read the docs fully). I expected, as part of indenting/formatting, that any unformatted variation of the same JSON input would produce same output.

It'd also be consistent with other funcs in encoding/json (like json.Encoder.Encode and json.MarshalIndent), as well as go/format.Source formatting, I believe.

Also, this is the additional code I needed to add to my code to produce expected/consistent output:

@@ -21,6 +21,14 @@ func run() error {

        var out bytes.Buffer
        err = json.Indent(&out, in, "", "\t")
        if err != nil {
                return err
        }
+       // TODO: Maybe not here; see https://github.com/golang/go/issues/13520.
+       // Add a newline, if there isn't one already.
+       if l := out.Len(); l != 0 && out.Bytes()[l-1] != '\n' {
+               err = out.WriteByte('\n')
+               if err != nil {
+                       return err
+               }
+       }

That wouldn't be needed if json.Indent behaved as documented.

I think I like the behavior as documented better, and would hope the implementation is fixed (since that would make json.Indent slightly better), but I'll leave to decide what's best overall to you.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2015

Well that's sad. It has been that way back at least to Go 1.2 (I don't have earlier Go versions on my Mac). It looks like all trailing spaces are preserved, not just a single newline.

Conflicted about changing this. Still thinking.

@dmitshur

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2015

It looks like all trailing spaces are preserved, not just a single newline.

You're right. In that case, I'll need to change my code to be quite non-trivial in order to be able to use the result of json.Indent in a predictable manner (i.e., always having a single trailing newline), and do so in an efficient manner (i.e., avoiding copying memory ala strings.TrimSpace(out.String())):

    ...

    var out bytes.Buffer
    err = json.Indent(&out, in, "", "\t")
    if err != nil {
        return err
    }
    // TODO: Maybe not here; see https://github.com/golang/go/issues/13520.
    // Trim all whitespace at the end. Then add a single newline.
    i := trimRightSpace(out.Bytes())
    out.Truncate(i)
    err = out.WriteByte('\n')
    if err != nil {
        return err
    }

    ...
}

// trimRightSpace returns index i, such that s[0:i] is a subslice of s
// by slicing off all trailing white space, as defined by Unicode.
func trimRightSpace(s []byte) int {
    i := lastIndexFunc(s, unicode.IsSpace, false)
    if i >= 0 && s[i] >= utf8.RuneSelf {
        _, wid := utf8.DecodeRune(s[i:])
        i += wid
    } else {
        i++
    }
    return i
}

// lastIndexFunc is the same as bytes.LastIndexFunc except that if
// truth==false, the sense of the predicate function is inverted.
func lastIndexFunc(s []byte, f func(r rune) bool, truth bool) int {
    for i := len(s); i > 0; {
        r, size := rune(s[i-1]), 1
        if r >= utf8.RuneSelf {
            r, size = utf8.DecodeLastRune(s[0:i])
        }
        i -= size
        if f(r) == truth {
            return i
        }
    }
    return -1
}

Here's an updated playground link:

https://play.golang.org/p/pJpGqNhgz7

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2015

Sure, that works. So does:

err = json.Indent(&out, bytes.TrimSpace(in), "", "\t")
out.WriteByte('\n')

@rsc rsc closed this in 1be2ddd Dec 8, 2015

dmitshur added a commit to shurcooL/cmd that referenced this issue Dec 13, 2015

@golang golang locked and limited conversation to collaborators Dec 14, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.