andrewmcveigh / cljs-time

A clj-time inspired date library for clojurescript.
342 stars 57 forks source link

Week number in year is wrong #51

Closed featalion closed 8 years ago

featalion commented 9 years ago

Hi, I found an issue with formatters, like :weekyear-week. It spreads across all formatters, that uses ww (week number). Current implementation does not follow ISO 8601.

It is actually very easy to fix the issue, because Google Closure library is already implement required method. Just change this line to:

  "ww" #(.getWeekNumber %)
andrewmcveigh commented 9 years ago

Hi,

So I've fixed the "ww" formatter as you suggest, but I've discovered that even that implementation is not correct.

Getting round to fixing "xxxx" & "ww" as per https://en.wikipedia.org/wiki/ISO_8601#Week_dates

Thanks for reporting!

featalion commented 8 years ago

This function seems to work fine for me:

(defn week-of-year
  [date & {:keys [cut-off-day first-week-day] :or {cut-off-day 3, first-week-day 0}}]
  (goog/date.getWeekNumber (.getFullYear date)
                           (.getMonth date)
                           (.getDate date)
                           cut-off-day
                           first-week-day))

Closure library API documentation on Date

pepijn commented 8 years ago

https://closure-library.googlecode.com/git-history/docs/local_closure_goog_i18n_datetimeformat.js.source.html:

 * Symbol Meaning Presentation        Example
 * ------   -------                 ------------        -------
 * G        era designator          (Text)              AD
 * y#       year                    (Number)            1996
 * Y*       year (week of year)     (Number)            1997
 * u*       extended year           (Number)            4601
[...]

 * Item marked with '*' are not supported yet.

Does the statement above make clear that week years (Y*) are not supported in the Google Closure library at all? Can't have a proper weekyear-week without a week year, right?

andrewmcveigh commented 8 years ago

I have a function for week/weekyear in a branch, seems to work but not extensively tested yet.

https://github.com/andrewmcveigh/cljs-time/blob/wip/real-parser/src/cljs_time/internal/unparse.cljs#L87

andrewmcveigh commented 8 years ago

This should have been fixed in [com.andrewmcveigh/cljs-time "0.5.0-alpha1"], closing.

https://github.com/andrewmcveigh/cljs-time/blob/master/src/cljs_time/internal/unparse.cljs#L87