code review 7393075: strconv: Use Quote to escape the input string for faile...

strconv: Use Quote to escape the input string for failed conversion errors.

This reveals the presence of control and non-printable characters in the
errors returned by the Parse functions.  Also add unit tests for NumError.

[mdbrown, 2013-02-27 21:08:04]

Hello golang-dev@googlegroups.com,

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

[r, 2013-02-27 21:32:47]

https://codereview.appspot.com/7393075/diff/1001/src/pkg/strconv/atoi_test.go
File src/pkg/strconv/atoi_test.go (right):

https://codereview.appspot.com/7393075/diff/1001/src/pkg/strconv/atoi_test.go...
src/pkg/strconv/atoi_test.go:192: for _, c := range []struct{ Num, Want string
}{
the other tests here don't use this style. please be consistent.

[rsc, 2013-02-27 21:40:41]

Please call CanBackquote first and preserve the current `` quoting where
possible.

[mdbrown, 2013-02-27 23:01:18]

Russ, do you mean something like

        if CanBackquote(e.Num) {
                quoted = "`" + e.Num + "`"
        } else {
                quoted = Quote(e.Num)
        }

?


On Wed, Feb 27, 2013 at 1:40 PM, Russ Cox <rsc@golang.org> wrote:

> Please call CanBackquote first and preserve the current `` quoting where
> possible.
>
>

[rsc, 2013-02-27 23:04:09]

Yes, please. I think that will preserve the current string format except
when there are backquotes or control characters.

[mdbrown, 2013-02-27 23:13:42]

I may not be following correctly but I think it has somewhat the opposite
effect. Simple strings can be back-quoted, yielding test failures from
other packages like

--- FAIL: TestNumberAccessors (0.00 seconds)
        decode_test.go:515: Number("-1.23e1").Int64() wanted error
"strconv.ParseInt: parsing \"-1.23e1\": invalid syntax" but got:
strconv.ParseInt: parsing `-1.23e1`: invalid syntax
        decode_test.go:515: Number("1e1000").Int64() wanted error
"strconv.ParseInt: parsing \"1e1000\": invalid syntax" but got:
strconv.ParseInt: parsing `1e1000`: invalid syntax
        decode_test.go:520: Number("1e1000").Float64() wanted error
"strconv.ParseFloat: parsing \"1e1000\": value out of range" but got:
strconv.ParseFloat: parsing `1e1000`: value out of range
FAIL
FAIL    encoding/json   0.023s


On Wed, Feb 27, 2013 at 3:04 PM, Russ Cox <rsc@golang.org> wrote:

> Yes, please. I think that will preserve the current string format except
> when there are backquotes or control characters.
>
>
>

[rsc, 2013-02-28 03:18:17]

My apologies for the wild goose chase. I misread this line:

-       return "strconv." + e.Func + ": " + `parsing "` + e.Num + `": ` +
e.Err.Error()

[mdbrown, 2013-02-28 16:55:01]

I've changed back to using a single call to Quote.

https://codereview.appspot.com/7393075/diff/1001/src/pkg/strconv/atoi_test.go
File src/pkg/strconv/atoi_test.go (right):

https://codereview.appspot.com/7393075/diff/1001/src/pkg/strconv/atoi_test.go...
src/pkg/strconv/atoi_test.go:192: for _, c := range []struct{ Num, Want string
}{
On 2013/02/27 21:32:47, r wrote:
> the other tests here don't use this style. please be consistent.

Done.

[mdbrown, 2013-02-28 16:55:24]

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

Please take another look.

[r, 2013-02-28 18:00:53]

LGTM

[r, 2013-02-28 18:08:07]

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

strconv: use Quote to escape the input string for failed conversion errors

This reveals the presence of control and non-printable characters in the
errors returned by the Parse functions.  Also add unit tests for NumError.

R=golang-dev, r, rsc
CC=golang-dev
https://codereview.appspot.com/7393075

Committer: Rob Pike <r@golang.org>