mde / timezone-js

DEPRECATED: Timezone-enabled JavaScript Date object. Uses Olson zoneinfo files for timezone data.
824 stars 183 forks source link

Update date.js #124

Closed ecanner closed 10 years ago

ecanner commented 10 years ago

Line 211 is now checking for a minus sign so that a string similar to Etc/GMT-12 will correctly evaluate as a timezone

mde commented 10 years ago

Could you include a test for this?

ecanner commented 10 years ago

Certainly

There is a test at https://mapmakerpro.com/projectmanager/tztest.html

If you need another example, let me know.

Ethan On Jan 2, 2014 8:17 PM, "Matthew Eernisse" notifications@github.com wrote:

Could you include a test for this?

— Reply to this email directly or view it on GitHubhttps://github.com/mde/timezone-js/pull/124#issuecomment-31499291 .

mde commented 10 years ago

No, I meant, can you add a test to the test suite for this. We want to make sure that doesn't get regressed if we make further changes.

ecanner commented 10 years ago

My apologies. I will get that done. On Jan 2, 2014 9:00 PM, "Matthew Eernisse" notifications@github.com wrote:

No, I meant, can you add a test to the test suite for this. We want to make sure that doesn't get regressed if we make further changes.

— Reply to this email directly or view it on GitHubhttps://github.com/mde/timezone-js/pull/124#issuecomment-31500854 .

gautema commented 10 years ago

I had the same problem, and this fix did the trick for me. It would be nice if you could merge it to the master branch so I don't need to keep my own fork of this project.

longlho commented 10 years ago

@gautema Can you include your use case here? I'll write some tests for it w/ this fix

gautema commented 10 years ago

@longlho My use case is that my Olson timezone file contains zone like this: "Etc/GMT-5":[[-300,"-","GMT-5",null]] This is not working until I apply the patch from @ecanner. The zone just create an unvalid date, and gives no good error message.

longlho commented 10 years ago

hmm kk can you give me something specific like new timezoneJS.Date(1234) & it gives you an invalid result?

gautema commented 10 years ago

I wrote these two tests in date.spec.js to demonstrate. The first one does not pass, while the second does.

it('should be able to set hours on GMT-5', function () {
    var dtA = new Date(0)
    , dt = new timezoneJS.Date(0, 'Etc/GMT-5');

    dt.setHours(6);
    expect(dt.getHours()).toEqual(6);
  });    
  it('should be able to set hours on GMT+5', function () {
    var dtA = new Date(0)
    , dt = new timezoneJS.Date(0, 'Etc/GMT+5');

    dt.setHours(6);
    expect(dt.getHours()).toEqual(6);
  });
longlho commented 10 years ago

Awesome that's very helpful :) I'll try to take a look as soon as I can

gautema commented 10 years ago

I forked the repo, added a test for it and merged inn ecanner's code. It's located at https://github.com/gautema/timezone-js. Take a look if you like to.

gautema commented 10 years ago

Added pull request #130. Hopefully this will make it a bit easier.