Skip to content

Commit

Permalink
Make week number parsing conform to ISO8601 when Datetime::weekstart …
Browse files Browse the repository at this point in the history 
…== 1

This is a duplicate of the changes from commit with same subject line in
GothenburgBitFactory/libshared#81 and also duplicates the commit
message below.  Timewarrior has copies of many functions from
libshared's Datetime class in its own DatetimeParser class, and until
such time as these classes are integrated, we have to maintain copies of
these functions here, and the changes need to be duplicated.  See
discussion in aforementioned PR.

The one difference with the patch over there is, this one is using the
public Datetime::dayOfWeek rather than its own implementation.  The copy
in Timewarrior already was doing this before, but it's worth noting it's
the only difference with the corresponding patch in libshared PR 81.

                           *     *    *

This patch makes the parsing of week numbers in dates ISO-8601 compliant
in the case that Datetime::weekstart == 1, while the existing behavior
remains available if Datetime::weekstart == 0.

The previous code parsed week numbers (given as "yyyy-Www") into dates
starting on Sunday.  Although the code had a "Datetime::weekstart"
variable, and this value was initialized to 1 (which is Monday) in
libshared, nonetheless week specifications would be parsed into calendar
days starting on Sunday.

Furthermore, week days in such given weeks ('d' in "yyyy-Www-d") used 0-6
for Sunday-Monday, while ISO8601 specifies 1-7 Monday-Sunday.
Therefore, yyyy-Www-0 was treated as valid (and should not be), while
yyyy-Www-7 was invalid (and should be valid).

Note that neither Taskwarrior nor Timewarrior ever set the value of
Datetime::weekstart.  Taskwarrior has an rc.weekstart, but this is only
used for "task calendar" output, not for parsing dates.

The patch does the following:

- Initialize "Datetime" instances with a weekday value from
  Datetime::weekstart.  This helps the case when weekday is not
  supplied, it won't default to zero and fail validation (when
  weekstart is '1').  Note that mktime() is usually used in the code
  to convert populated "struct tm" broken-down times into epoch-times,
  and this operation does not use tm.tm_wday for input, only as an
  output field, recomputed as a normalized value, so it appears to be
  safe to initialize it with a 1 (which we might wonder about since
  .tm_wday is supposed to be 0-6 Sunday based).

- Use the already-existing Datetime::parse_weekday() to parse the
  'ww' in "yyyy-Www" input dates (the function was not used by any
  caller previously; guessing it may have been intended for eventual
  use in order to respect weekstart(?))

- Teach parse_weekday() about weekstart.  Previously this assumed
  1-7, which is the reason I'm guessing this was intended to be used
  for ISO weeks eventually.  Now it can also use 0-6 if weekstart 0.

- Teach Datetime::validate to respect Datetime::weekstart also.
  Previously only 0-6 Sunday-Monday was considered valid.

- Default the weekday to Datetime::weekstart if not supplied, ie for
  "yyyy-Www-X" if the "-X" is not supplied, as recognized when
  Datetime::standaloneDateEnabled = true, which is the case for (1)
  tests, (2) timewarrior, but NOT for taskwarrior at this time
  (both the standalone 'calc' and 'task calc' (ie Context.cpp) set
  this to false).

- Implement the complete ISO week calculation algorithm in
  Datetime::resolve() if weekstart is '1', and keeps the existing
  algorithm if weekstart is '0'.  This will allow Taskwarrior and
  Timewarrior to offer the option of the old behavior for those
  who want to use Sunday-based weeks and ignore ISO week calculations
  (such as 2023-01-01 being in ISO week 2022-W52).

Signed-off-by: Scott Mcdermott <[email protected]>
  • Loading branch information
@smemsh
smemsh committed Aug 14, 2024
1 parent 42ef526 commit c9e4444
Showing 1 changed file with 61 additions and 14 deletions.
75 changes: 61 additions & 14 deletions src/DatetimeParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void DatetimeParser::clear ()
_year = 0;
_month = 0;
_week = 0;
_weekday = 0;
_weekday = Datetime::weekstart;
_julian = 0;
_day = 0;
_seconds = 0;
Expand Down Expand Up @@ -462,18 +462,22 @@ bool DatetimeParser::parse_date_ext (Pig& pig)
int month {};
int day {};
int julian {};
int week {};
int weekday {};

if (pig.skip ('W') &&
parse_week (pig, _week))
parse_week (pig, week))
{
if (pig.skip ('-') &&
pig.getDigit (_weekday))
if (! (pig.skip ('-') &&
parse_weekday (pig, weekday)))
{
// What is happening here - must be something to do?
weekday = Datetime::weekstart;
}

if (! unicodeLatinDigit (pig.peek ()))
{
_weekday = weekday;
_week = week;
_year = year;
return true;
}
Expand Down Expand Up @@ -679,13 +683,14 @@ bool DatetimeParser::parse_date (Pig& pig)
if (pig.skip ('W') &&
parse_week (pig, week))
{
if (pig.getDigit (weekday))
_weekday = weekday;
if (! (parse_weekday (pig, weekday)))
weekday = Datetime::weekstart;

if (! unicodeLatinDigit (pig.peek ()))
{
_year = year;
_week = week;
_weekday = weekday;
return true;
}
}
Expand Down Expand Up @@ -926,8 +931,8 @@ bool DatetimeParser::parse_weekday (Pig& pig, int& value)

int weekday;
if (pig.getDigit (weekday) &&
weekday >= 1 &&
weekday <= 7)
weekday >= Datetime::weekstart &&
weekday <= (Datetime::weekstart == 1 ? 7 : 6))
{
value = weekday;
return true;
Expand Down Expand Up @@ -2786,11 +2791,15 @@ bool DatetimeParser::isOrdinal (const std::string& token, int& ordinal)
// Validation via simple range checking.
bool DatetimeParser::validate ()
{
// for ISO 8601 week specification: YYYY-Www-D where D is 1-7 (Mon-Sun)
int wks = (Datetime::weekstart == 1 ? 1: 0);
int wke = (Datetime::weekstart == 1 ? 7: 6);

// _year;
if ((_year && (_year < 1900 || _year > 9999)) ||
(_month && (_month < 1 || _month > 12)) ||
(_week && (_week < 1 || _week > 53)) ||
(_weekday && (_weekday < 0 || _weekday > 6)) ||
(true && (_weekday < wks || _weekday > wke)) ||
(_julian && (_julian < 1 || _julian > Datetime::daysInYear (_year))) ||
(_day && (_day < 1 || _day > Datetime::daysInMonth (_year, _month))) ||
(_seconds && (_seconds < 1 || _seconds > 86400)) ||
Expand All @@ -2807,7 +2816,7 @@ bool DatetimeParser::validate ()
// int tm_mday; day of month (1 - 31)
// int tm_mon; month of year (0 - 11)
// int tm_year; year - 1900
// int tm_wday; day of week (Sunday = 0)
// int tm_wday; day of week (0-6 or 1-7 depending on Datetime::weekstart)
// int tm_yday; day of year (0 - 365)
// int tm_isdst; is daylight saving time in effect?
// char *tm_zone; abbreviation of timezone name
Expand Down Expand Up @@ -2851,16 +2860,54 @@ void DatetimeParser::resolve ()
month == 0 &&
day == 0 &&
week == 0 &&
weekday == 0 &&
weekday == Datetime::weekstart &&
seconds < seconds_now)
{
seconds += 86400;
}

// Convert week + weekday --> julian.
// Convert week + weekday --> julian, possibly in the previous or next
// calendar year (for ISO8601 weeks). Weekday will be 1-7 if weekstart = 1,
// as per ISO week spec. This is incompatible with tm.tm_wday (which is
// 0-6, Sunday based), but that field is not assigned anywhere in this
// function, and note that mktime() uses tm_wday only as an output field.
// We use Datetime::weekstart as a hint about whether the user is
// giving/wants ISO weeks, although the very use of 2024-W30 and 2024-W30-1
// format is from ISO8601 in the first place.
//
if (week)
{
julian = (week * 7) + weekday - Datetime::dayOfWeek (year, 1, 4) - 3;
// Get 0-based (Sunday) and 1-based (Monday, ie ISO) dow for 4th Jan
// 0 Sun, 1 Mon, 2 Tue, 3 Wed, 4 Thu, 5 Fri, 6 Sat
// 1 Mon, 2 Tue, 3 Wed, 4 Thu, 5 Fri, 6 Sat, 7 Sun
//
int jan4dow0 = Datetime::dayOfWeek (year, 1, 4);
int jan4dow1 = (jan4dow0 == 0 ? 7 : jan4dow0);

if (Datetime::weekstart == 1)
{
// https://en.wikipedia.org/wiki/ISO_week_date
julian = week * 7 + weekday - (jan4dow1 + 3);
if (julian < 1)
{
julian += daysInYear (--year);
}
else
{
int yeardays = daysInYear (year);
if (julian > yeardays)
{
year++;
julian -= yeardays;
}
}
}
else
{
// Prior algorithm that was used for all weekstarts before, now only
// used for Sunday weekstarts
julian = week * 7 + weekday - jan4dow0 - 3;
}
}

// Provide default values for year, month, day.
Expand Down

0 comments on commit c9e4444

Please sign in to comment.