code review 7288052: time: deal a bit better with time zones in Parse

time: deal a bit better with time zones in Parse

* Document Parse's zone interpretation.
* Add ParseInLocation (API change).
* Recognize "wrong" time zone names, like daylight savings time in winter.
* Disambiguate time zone names using offset (like winter EST vs summer EST in Sydney).

The final two are backwards-incompatible changes, but I believe
they are both buggy behavior in the Go 1.0 versions; the old results
were more wrong than the new ones.

Fixes issue 3604.
Fixes issue 3653.
Fixes issue 4001.

[rsc, 2013-02-04 02:04:07]

Hello golang-dev@googlegroups.com,

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

[adg, 2013-02-04 02:43:07]

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/example_test.go
File src/pkg/time/example_test.go (right):

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/example_test.go...
src/pkg/time/example_test.go:71: const long = "Jan 2, 2006 at 3:04pm (MST)"
It's not clear to me which of these is the format string and which is the string
to be parsed. Perhaps use two constants descriptively named?

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go
File src/pkg/time/time_test.go (right):

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:565: t.Fatalf("ParseInLocation(Feb 01 2013 EST,
Sydney) = %v, want %v", t1, t2)
s/Feb/Aug/

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:568: if offset != 10*60*60 {
if want := 10*60*60; offset != want {

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:569: t.Fatalf("ParseInLocation(Feb 01 2013 EST,
Sydney).Zone = _, %d, want _, %d", offset, 10*60*60)
s/Feb/Aug/
s/10*60*60/want/

[rsc, 2013-02-04 03:46:40]

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

Please take another look.

[rsc, 2013-02-04 03:46:55]

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/example_test.go
File src/pkg/time/example_test.go (right):

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/example_test.go...
src/pkg/time/example_test.go:71: const long = "Jan 2, 2006 at 3:04pm (MST)"
On 2013/02/04 02:43:07, adg wrote:
> It's not clear to me which of these is the format string and which is the
string
> to be parsed. Perhaps use two constants descriptively named?

Remember that this example will be showing immediately under the docs for the
method. I had hoped that the use of a const at all would suggest 'format
string', because if the thing you're parsing was a constant you wouldn't need to
use Parse in the first place. To emphasize this I renamed the constants to
longForm and shortForm (instead of long and short). I'd prefer not to introduce
a constant for the variable part of the call, for the reason just given.

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go
File src/pkg/time/time_test.go (right):

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:565: t.Fatalf("ParseInLocation(Feb 01 2013 EST,
Sydney) = %v, want %v", t1, t2)
On 2013/02/04 02:43:07, adg wrote:
> s/Feb/Aug/

Done.

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:568: if offset != 10*60*60 {
On 2013/02/04 02:43:07, adg wrote:
> if want := 10*60*60; offset != want {

Thanks, but no thanks. I think that form is overused. It's just as easy to see
that it says 10*60*60 in both places as it is to see that it says 'want' in both
places. For trivial cases the indirection hurts readability more than it helps.

https://codereview.appspot.com/7288052/diff/2001/src/pkg/time/time_test.go#ne...
src/pkg/time/time_test.go:569: t.Fatalf("ParseInLocation(Feb 01 2013 EST,
Sydney).Zone = _, %d, want _, %d", offset, 10*60*60)
On 2013/02/04 02:43:07, adg wrote:
> s/Feb/Aug/
> s/10*60*60/want/

Done half.

[adg, 2013-02-04 03:50:59]

LGTM

[rsc, 2013-02-04 04:02:17]

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

time: deal a bit better with time zones in Parse

* Document Parse's zone interpretation.
* Add ParseInLocation (API change).
* Recognize "wrong" time zone names, like daylight savings time in winter.
* Disambiguate time zone names using offset (like winter EST vs summer EST in
Sydney).

The final two are backwards-incompatible changes, but I believe
they are both buggy behavior in the Go 1.0 versions; the old results
were more wrong than the new ones.

Fixes issue 3604.
Fixes issue 3653.
Fixes issue 4001.

R=adg
CC=golang-dev
https://codereview.appspot.com/7288052