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

Already on GitHub? Sign in to your account

Add support for parsing weekdays and week numbers. #764

Closed
wants to merge 1 commit into
from

Conversation

3 participants
Collaborator

jasondavies commented Aug 13, 2012

Also includes support for parsing "%" literals.

@mbostock mbostock and 1 other commented on an outdated diff Aug 13, 2012

src/time/format.js
@@ -28,8 +28,16 @@ d3.time.format = function(template) {
// The am-pm flag is 0 for AM, and 1 for PM.
if ("p" in d) d.H = d.H % 12 + d.p * 12;
- var date = new d3_time();
- date.setFullYear(d.y, d.m, d.d);
+ var date = new d3_time(0);
+ if ("j" in d) {
+ date.setFullYear(d.y, 0, 1);
+ date.setTime(+date + d.j * 864e5);
@mbostock

mbostock Aug 13, 2012

Owner

Does this work correctly with daylight savings because we later call setHours?

@jasondavies

jasondavies Aug 13, 2012

Collaborator

Good point! It turns out this is error-prone for some time zones e.g.:

var d = new Date(0);
d.setFullYear(2012, 0, 1);
d.setTime(+d + 100 * 864e5);
console.log(d);
$ node tz.js 
Tue Apr 10 2012 01:00:00 GMT+0100 (BST)
$ TZ=America/Scoresbysund node tz.js 
Wed Apr 11 2012 00:00:00 GMT+0000 (EGST)

Rather than fiddling with offsets directly, it turns out there's an elegant solution: simply pass the full day-of-year to setFullYear and it will automatically wrap to the correct month and day.

@jasondavies

jasondavies Aug 13, 2012

Collaborator

I've fixed a similar issue for week-of-year/day-of-week parsing too.

Collaborator

jasondavies commented Apr 24, 2013

Rebased against version 3.1.6.

can this pull request be merged ?

Add support for parsing weekdays and week numbers.
Also includes support for parsing "%" literals.
Collaborator

jasondavies commented Jun 8, 2013

Can we include this in 3.2? It seems like a simple enough change.

Owner

mbostock commented Jun 8, 2013

Sure, that sounds reasonable. I haven’t closely examined this pull request, but as a skim read, it looks good.

Collaborator

jasondavies commented Jun 8, 2013

Excellent, thanks. Staged in #1285 for 3.2.

@jasondavies jasondavies closed this Jun 8, 2013

@jasondavies jasondavies deleted the unknown repository branch Jun 8, 2013

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