date/strftime #83

Merged
merged 7 commits into from May 22, 2013

Projects

None yet

3 participants

@millermedeiros
Member

started date/strftime implementation.

following the tokens present on python, ruby and C

still missing some stuff but basic features are working.. I also added an internal date/i18n_ module to allow setting the locale globally but I guess we need another option to override the default locale as well (maybe 3rd argument).

see #78

@millermedeiros millermedeiros commented on an outdated diff Apr 8, 2013
tests/spec/date/spec-strftime.js
+ }
+ }, '%z') ).toBe('-0330');
+ expect( strftime({
+ getTimezoneOffset : function(){
+ return -210;
+ }
+ }, '%z') ).toBe('+0330');
+ });
+
+ it('should support escaping %%', function () {
+ expect( strftime(date, '%%') ).toEqual('%');
+ });
+
+ it('should support multiple tokens at once', function () {
+ // iso8601
+ expect( strftime(date, '%Y-%m-%dT%H:%M:%S%z') ).toEqual( '2013-04-08T09:02:04-0300' );
@millermedeiros
millermedeiros Apr 8, 2013 Member

ops, we need to remove the %z otherwise test will fail if timezoneOffset is different..

@millermedeiros millermedeiros commented on an outdated diff Apr 8, 2013
tests/spec/date/spec-strftime.js
+ it('should return zero-padded minutes for %M', function () {
+ expect( strftime(date, '%M') ).toBe('02');
+ expect( strftime(date_2, '%M') ).toBe('22');
+ });
+
+ it('should return newline char for %n', function () {
+ expect( strftime(date, '%n') ).toBe('\n');
+ });
+
+ it('should return am/pm for %p', function () {
+ expect( strftime(date, '%p') ).toBe('am');
+ expect( strftime(date_2, '%p') ).toBe('pm');
+ });
+
+ it('should return number of seconds since epoch for %s', function () {
+ expect( strftime(date, '%s') ).toBe('1365422524');
@millermedeiros
millermedeiros Apr 8, 2013 Member

this will fail on travis server since timezone is different from my local machine.. we need to mock the date.

@millermedeiros
Member

I'm wondering if we should split each case into its own function (ie. date/getTimezone, date/getCentury, date/toSeconds, date/weekDayName, date/monthNameAbbr, etc..)

It will probably make things easier to reuse and can make code cleaner in some cases (eg. while building a calendar you probably need weekDayName and monthName, strftime(date, '%A') is not very readable..)

@millermedeiros
Member

implementation is covering all the tokens from http://pubs.opengroup.org/onlinepubs/007908799/xsh/strftime.html besides %V (ISO 8601 week number)

I extracted all the formats that aren't one-liners into separate modules and kept the localization info as a separate file, that way we could potentially create multiple i18n files inside the date package.

strftime is really cryptic, I think we should really implement LDML date format after strftime is done.

@millermedeiros
Member

I'm almost merging this without the ISO week number support (%V) since I will probably get the implementation wrong (or take a long time to do it right) and I never needed it. This PR is already holding the v0.6 release for too long...

Anyone against an incomplete implementation of strftime landing on v0.6? (with the option to improve it later if needed)

/cc @conradz @MathiasPaumgarten @leocavalcante

@MathiasPaumgarten
Member

Sorry, I've been a little lazy on this PR since it's quite a junk of code to read through. I have no objections at all. A welcomed feature for sure, I was just keeping quiet since I never got around to check out the PR closely. Shame on me.

Anyways 👍 and looking forward to 0.6.0!

@conradz
Member
conradz commented May 21, 2013

I would be OK with an incomplete implementation, as long as the incomplete parts (parts implemented in the C strftime and not implemented in this one) are documented. Probably only a few people need the %V.

@millermedeiros
Member

I rewrote the whole commit history (so it makes more sense) and added docs to all the new methods. I also marked the %V with a <del> (which will render with a strikethrough on the docs) so it should be enough for now.

I think it's good to merge, planning to do it tomorrow and release v0.6 after that.

@MathiasPaumgarten
Member

I will add a de-DE version once it's merged, so I can pull and create e PR.

@millermedeiros millermedeiros merged commit c2f4f42 into mout:master May 22, 2013

1 check passed

default The Travis CI build passed
Details
@millermedeiros millermedeiros deleted the millermedeiros:strftime branch May 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment