Issue 9568011: Unify Date.fromString and support more Iso 8601.

Issue 9568011: Unify Date.fromString and support more Iso 8601. (Closed)

Created:
8 years, 9 months ago by floitsch

Modified:
8 years, 9 months ago

Reviewers:
kasperl

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Unify Date.fromString and support more Iso 8601. Committed: https://code.google.com/p/dart/source/detail?r=4923

Patch Set 2 : Minor change (use trimmed). #

Patch Set 3 : Improve parseInt. #

Total comments: 12

Patch Set 4 : Rebase. #

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+388 lines, -129 lines)			Patch
M	frog/leg/lib/date_helper.dart	View		1 chunk	+9 lines, -0 lines	0 comments	Download
M	frog/leg/lib/js_helper.dart	View	1 2 3	1 chunk	+8 lines, -3 lines	0 comments	Download
M	frog/leg/lib/mockimpl.dart	View		3 chunks	+48 lines, -19 lines	0 comments	Download
M	frog/leg/ssa/codegen.dart	View	1 2 3	1 chunk	+3 lines, -0 lines	0 comments	Download
M	frog/lib/date_implementation.dart	View		6 chunks	+67 lines, -25 lines	0 comments	Download
M	frog/lib/math.dart	View		1 chunk	+4 lines, -1 line	0 comments	Download
M	runtime/lib/date.dart	View		7 chunks	+85 lines, -77 lines	0 comments	Download
M	tests/co19/co19-leg.status	View	1 2 3	2 chunks	+3 lines, -4 lines	0 comments	Download
M	tests/co19/co19-runtime.status	View	1 2 3	1 chunk	+3 lines, -0 lines	0 comments	Download
M	tests/corelib/src/DateTimeTest.dart	View		2 chunks	+158 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

kasperl

LGTM. https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/leg/lib/mockimpl.dart File frog/leg/lib/mockimpl.dart (right): https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/leg/lib/mockimpl.dart#newcode279 frog/leg/lib/mockimpl.dart:279: addOneMillisecond = ((milliseconds %100) >= 50); %100 -> ...

8 years, 9 months ago (2012-03-02 13:34:10 UTC) #2

floitsch

8 years, 9 months ago (2012-03-02 17:00:24 UTC) #3

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/leg/lib/mockimp...
File frog/leg/lib/mockimpl.dart (right):

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/leg/lib/mockimp...
frog/leg/lib/mockimpl.dart:279: addOneMillisecond = ((milliseconds %100) >= 50);
On 2012/03/02 13:34:10, kasperl wrote:
> %100 -> % 100

Done.

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/lib/date_implem...
File frog/lib/date_implementation.dart (right):

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/lib/date_implem...
frog/lib/date_implementation.dart:87: addOneMillisecond = ((milliseconds %100)
>= 50);
On 2012/03/02 13:34:10, kasperl wrote:
> %100 -> % 100

Done.

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/lib/math.dart
File frog/lib/math.dart (right):

https://chromiumcodereview.appspot.com/9568011/diff/1008/frog/lib/math.dart#n...
frog/lib/math.dart:54: var isHex = (str.length > 2) && str[0] == '0' &&
On 2012/03/02 13:34:10, kasperl wrote:
> Do you need to trim first?

Using a regexp now.

https://chromiumcodereview.appspot.com/9568011/diff/1008/runtime/lib/date.dart
File runtime/lib/date.dart (right):

https://chromiumcodereview.appspot.com/9568011/diff/1008/runtime/lib/date.dar...
runtime/lib/date.dart:43: years, month, day, hours, minutes, seconds,
milliseconds,
On 2012/03/02 13:34:10, kasperl wrote:
> Weird indentation of the parameters.

Done.

https://chromiumcodereview.appspot.com/9568011/diff/1008/runtime/lib/date.dar...
runtime/lib/date.dart:63: final RegExp re = const RegExp(
On 2012/03/02 13:34:10, kasperl wrote:
> Is there anyway we could be sharing more of this code between the VM and Leg?

Waiting for Josh' refactorings and will reevaluate then.

https://chromiumcodereview.appspot.com/9568011/diff/1008/runtime/lib/date.dar...
runtime/lib/date.dart:94: addOneMillisecond = ((milliseconds %100) >= 50);
On 2012/03/02 13:34:10, kasperl wrote:
> %100 -> % 100

seems to be a common mistake... ;)
done.

Expand Messages | Collapse Messages