code review 6267045: time: accept .999 in Parse

time: accept .999 in Parse

The recent shuffle in parsing formats exposed probably unintentional
behavior in time.Parse, namely that it was mostly ignoring ".99999"
in the format, producing the following behavior:

fmt.Println(time.Parse("03:04:05.999 MST", "12:00:00.888 PDT")) // error (.888 unexpected)
fmt.Println(time.Parse("03:04:05.999", "12:00:00")) // error (input too short)
fmt.Println(time.Parse("03:04:05.999 MST", "12:00:00 PDT"))  // ok (extra bytes on input make it ok)

http://play.golang.org/p/ESJ1UYXzq2

API CHANGE:

This CL makes all three examples valid: ".999" can match an
empty string or else a fractional second with at most nine digits.

Fixes issue 3701.

[rsc, 2012-06-04 16:34:38]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2012-06-04 16:36:48]

LGTM

API change approved

[r2, 2012-06-04 16:37:46]

relevant to issue 3523?

-rob

[rsc, 2012-06-04 16:43:42]

PTAL

The code is unchanged.  The description now reads:

	This CL makes all three examples valid: ".999" can match an
	empty string or else a fractional second with at most nine digits.

(It used to say at most the given number of digits.)

I went back and forth on this and forgot to update the description after
my last flip-flop. The issue is that

    05 accepts 12
    05 accepts 12.3
    05 accepts 12.345

    05.0 rejects 12
    05.0 accepts 12.3
    05.0 rejects 12.345

    05.9 accepts 12
    05.9 accepts 12.3
    05.9 ??? 12.345

It seemed better to be lax than to be picky: especially since layouts
are prohibited from putting other digits at the end of a .999,
interpreting the extras as more fractional second digits is the only
possible non-error interpretation.

Being lax makes 05.9 behave exactly like 05, instead of introducing a
third kind of format.

Russ

[r, 2012-06-04 16:52:17]

LGTM

http://codereview.appspot.com/6267045/diff/2001/src/pkg/time/format.go
File src/pkg/time/format.go (right):

http://codereview.appspot.com/6267045/diff/2001/src/pkg/time/format.go#newcod...
src/pkg/time/format.go:861: if len(value) < 2 || value[0] != '.' || value[1] <
'0' || value[1] > '9' {
rewrite for my peeve:

if len(value) < 2 || value[0] != '.' || value[1] < '0' || '9' < value[1]

[rsc, 2012-06-04 17:09:28]

*** Submitted as http://code.google.com/p/go/source/detail?r=4f82ce08466d ***

time: accept .999 in Parse

The recent shuffle in parsing formats exposed probably unintentional
behavior in time.Parse, namely that it was mostly ignoring ".99999"
in the format, producing the following behavior:

fmt.Println(time.Parse("03:04:05.999 MST", "12:00:00.888 PDT")) // error (.888
unexpected)
fmt.Println(time.Parse("03:04:05.999", "12:00:00")) // error (input too short)
fmt.Println(time.Parse("03:04:05.999 MST", "12:00:00 PDT"))  // ok (extra bytes
on input make it ok)

http://play.golang.org/p/ESJ1UYXzq2

API CHANGE:

This CL makes all three examples valid: ".999" can match an
empty string or else a fractional second with at most nine digits.

Fixes issue 3701.

R=r, r
CC=golang-dev
http://codereview.appspot.com/6267045

[jgold.bg, 2012-06-04 17:53:20]

FWIW, I just synced and rebuilt and can confirm that this addresses my original
problem from 3523.

jonathan

On Mon, Jun 04, 2012 at 09:37:43AM -0700, Rob 'Commander' Pike wrote:
> relevant to issue 3523?
> 
> -rob