Add support for timezone offset and timezone name.

Add support for timezone offset and timezone name.

Committed: https://code.google.com/p/dart/source/detail?r=7801

[floitsch, 2012-05-16 20:47:13]

Still missing test cases, but here in the DK office Thursday and Friday are
vacation days, and I wanted to send you the review before the long weekend so
that you have more time to look at it.

[Sean Eagan, 2012-05-17 13:24:39]

https://chromiumcodereview.appspot.com/10383218/diff/1012/corelib/src/date.dart
File corelib/src/date.dart (right):

https://chromiumcodereview.appspot.com/10383218/diff/1012/corelib/src/date.da...
corelib/src/date.dart:130: Duration get timeZoneOffset();

Did you consider adding an 'isDayLightSavingsTime' or similar here ?  The
implementation could look something like:

bool get isDayLightSavingsTime() {
  final offset = timeZoneOffset.inMilliseconds;  
  return 
    offset < new Date(year   ).timeZoneOffset.inMilliseconds || 
    offset < new Date(year, 6).timeZoneOffset.inMilliseconds;
}

(would be handy if Duration supported comparison operators here)

I adapted this from:

https://github.com/timrwood/moment/blob/master/moment.js#L802

[Sean Eagan, 2012-05-17 13:47:19]

On 2012/05/17 13:24:39, seaneagan1 wrote:
> bool get isDayLightSavingsTime() {
>   final offset = timeZoneOffset.inMilliseconds;  
>   return 
>     offset < new Date(year   ).timeZoneOffset.inMilliseconds || 
>     offset < new Date(year, 6).timeZoneOffset.inMilliseconds;
> }

actually it should use > instead of < since the offset is inverted here vs.
JavaScript.

[floitsch, 2012-05-21 12:18:51]

On 2012/05/17 13:47:19, seaneagan1 wrote:
> On 2012/05/17 13:24:39, seaneagan1 wrote:
> > bool get isDayLightSavingsTime() {
> >   final offset = timeZoneOffset.inMilliseconds;  
> >   return 
> >     offset < new Date(year   ).timeZoneOffset.inMilliseconds || 
> >     offset < new Date(year, 6).timeZoneOffset.inMilliseconds;
> > }
> 
> actually it should use > instead of < since the offset is inverted here vs.
> JavaScript.

I think I will add a daylight savings getter, but not in this CL.

[Mads Ager (google), 2012-05-21 12:53:28]

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:67: int GetDaylightSavingOffsetInSeconds() {
static int GetDaylightSavingsOffsetInSeconds() {

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:68: TzSet();
Is the call to TzSet needed here?

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:79: bool OS::GetTimeZoneName(int64_t seconds_since_epoch,
There is a TzSet() call missing in this method I think. Using _tzname without
calling _tzset?

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:99: if (inDaylightSavingsTime != 0 && inDaylightSavingsTime
!= 1) return false;
Either a one-liner above or split into lines with braces here as above.

[floitsch, 2012-05-21 13:26:44]

PTAL.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:67: int GetDaylightSavingOffsetInSeconds() {
On 2012/05/21 12:53:28, Mads Ager wrote:
> static int GetDaylightSavingsOffsetInSeconds() {

Done.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:68: TzSet();
On 2012/05/21 12:53:28, Mads Ager wrote:
> Is the call to TzSet needed here?

No. Apparently not. Removed.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:79: bool OS::GetTimeZoneName(int64_t seconds_since_epoch,
On 2012/05/21 12:53:28, Mads Ager wrote:
> There is a TzSet() call missing in this method I think. Using _tzname without
> calling _tzset?

Updated comment: LocalTime implicitly calls tzset.

Therefore could also remove TzSet in GetTimeZoneOffsetInSeconds, and as a
consequence the whole TzSet function.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:99: if (inDaylightSavingsTime != 0 && inDaylightSavingsTime
!= 1) return false;
On 2012/05/21 12:53:28, Mads Ager wrote:
> Either a one-liner above or split into lines with braces here as above.

Done.

[floitsch, 2012-05-21 13:26:44]

PTAL.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:67: int GetDaylightSavingOffsetInSeconds() {
On 2012/05/21 12:53:28, Mads Ager wrote:
> static int GetDaylightSavingsOffsetInSeconds() {

Done.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:68: TzSet();
On 2012/05/21 12:53:28, Mads Ager wrote:
> Is the call to TzSet needed here?

No. Apparently not. Removed.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:79: bool OS::GetTimeZoneName(int64_t seconds_since_epoch,
On 2012/05/21 12:53:28, Mads Ager wrote:
> There is a TzSet() call missing in this method I think. Using _tzname without
> calling _tzset?

Updated comment: LocalTime implicitly calls tzset.

Therefore could also remove TzSet in GetTimeZoneOffsetInSeconds, and as a
consequence the whole TzSet function.

https://chromiumcodereview.appspot.com/10383218/diff/4002/runtime/vm/os_win.c...
runtime/vm/os_win.cc:99: if (inDaylightSavingsTime != 0 && inDaylightSavingsTime
!= 1) return false;
On 2012/05/21 12:53:28, Mads Ager wrote:
> Either a one-liner above or split into lines with braces here as above.

Done.

[Mads Ager (google), 2012-05-21 13:45:35]

LGTM