Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

week number is not compliant with ISO standard #613

Closed
thesmart opened this issue Feb 8, 2013 · 13 comments
Closed

week number is not compliant with ISO standard #613

thesmart opened this issue Feb 8, 2013 · 13 comments

Comments

@thesmart
Copy link

thesmart commented Feb 8, 2013

I found a bug today, assuming that moment('Jan 1, 2005').format('YYYY-\\Www-d') would follow the ISO 8601 standard. The code sample would return:

> moment('Jan 1, 2005').format('YYYY-\\Www-d')
'2005-W53-6'

but this is the expected output:

'2004-W53-6'

http://en.wikipedia.org/wiki/ISO_week_date#Examples

@timrwood
Copy link
Member

timrwood commented Feb 8, 2013

Check out #503. We are adding W WW Wo tokens for ISO week dates in version 2.0.0, which should be released shortly.

@thesmart
Copy link
Author

thesmart commented Feb 8, 2013

You guys rule, thanks

@thesmart thesmart closed this as completed Feb 8, 2013
@thesmart thesmart reopened this Feb 15, 2013
@thesmart
Copy link
Author

Reopening this issue because v2.0.0 still doesn't have it quite right.

The ISO standard is to include the ISO week year. The above code example, ported to v2.0.0, looks like:

> moment('Jan 1, 2005').format('YYYY-\\WWW-d')
'2005-W53-6'

this now produces:

'2005-WW53-6'

Notice, there are two bugs here. 1) An extra "W" is emitted. 2) the Year is not correct. It should be:

'2004-W53-6'

@ichernev
Copy link
Contributor

So you suggest we find a iso-week format token and modify an year token in the same string to match iso-week-related year? I guess we need iso-week-year to go in hand with the week number for all this to make sense.

@ichernev
Copy link
Contributor

For the 'W' issue, use format('[W]W') as a single W is supposed to mean iso-week, and escaping is done in square brackets.

@timrwood
Copy link
Member

Regarding the ISO week year, we should probably add iso week year and iso week day.

What about the following for the api?

moment.fn.weekday // like moment.fn.day, but with the locale based week start
moment.fn.isoWeekday // like moment.fn.day, but with iso based week start
moment.fn.weekYear // locale based week year 
moment.fn.isoWeekYear // iso based week year

As for the tokens, that's where it gets messy. LDML uses YYYY for the iso week year, and yyyy for the month+date year. If we use yyyy for the iso year, we would be a complete inversion of LDML.

For reference, strftime uses G/g and php date uses o.

Maybe we could use GGGG for ISO week year, and gggg for non-iso week year?

For the day of the week, what about e/E? This is what LDML uses for days of the week.

So the format that the OP would need would be GGGG-[W]WW-E for the iso week, and gggg-[W]ww-e for the non-iso week.

@thesmart
Copy link
Author

@timrwood love the proposal for adding iso week year api methods.

I figure if someone wants the week year in a formatted string, its likely that they will be building the entire ISO week-year format. Therefore, perhaps a single token for the ISO standard week-year format is enough?

@timrwood
Copy link
Member

Hmm, adding a single token to mirror GGGG-[W]WW-E kinda breaks the one token to one date part paradigm (except for the localized formats).

Additionally, if I'm reading the ISO standard correctly, the ISO week representation could be any of the following.

2012W03
2012-W03
2012W036
2012-W03-6
2012-W03-6T12
2012-W03-6T12:30
2012-W03-6T12:30:45
2012-W03-6T12:30:45.389

If you want, you could always add a method to the moment.prototype so you don't have to duplicate GGGG-[W]WW-E everywhere.

moment.fn.isoWeekFormat = function () {
    return this.format('GGGG-[W]WW-E');
}

@ichernev
Copy link
Contributor

@timrwood so you suggest we keep YYYY and d but also add gggg, GGGG, e and E? If this will make moment more compliant with this LDML then maybe, otherwise I think its a bit too complicated.

@karljacuncha
Copy link

Just posting here in case my hack is useful to anybody.

I'm working on a projects that uses the YYYY-WWW format, so have shoe-horned a fix for that into the latest moment.js that works with all the examples on the wiki page: http://en.wikipedia.org/wiki/ISO_week_date.
moment("2013-W19")
gives:
Moment { ... _d: Mon May 06 2013 00:00:00 GMT+0100 (GMT Daylight Time) ... }

And
moment().format("YYYY-WWW")
gives:
"2013-W19"

Is a pretty blunt hack onto the constructor and an additional output format, but does the trick for this specific case/format.
Code in commits here:
karljacuncha@73baab1
karljacuncha@73baab1

@ichernev
Copy link
Contributor

The new release 2.1.0 supports (iso)?(week|weekday|weekyear) tokens/getters/setters.

@sahanDissanayake
Copy link

moment().week()
breaks when

moment.lang('zh-cn', { week : { dow : 1 // Monday is the first day of the week } });

this is done

@mysticman
Copy link

Here is the the solution, for the correct week days order, since the last version of moment.js

moment.weekdays(true);
moment.weekdaysShort(true);
moment.weekdaysMin(true);

https://jsfiddle.net/Chopperbp/pn79ptqx/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants