GothenburgBitFactory / libshared

Libshared - Common Code Submodule
Other
5 stars 20 forks source link

weeks seem to start on Sunday despite weekstart = 1 #80

Closed smemsh closed 3 months ago

smemsh commented 4 months ago

There seems to be an issue in this library regarding week numbers and starting day of week.

ISO weeks start on Monday. The code seems to agree:

  src/Datetime.cpp:62:int Datetime::weekstart = 1; // Monday, per ISO-8601.

In the tests from test/datetime.t.cpp there are some initialized variables:

  ...
  int year = 2013;
  int mo = 12;
  ...
  local_now->tm_year  = year - 1900;
  local_now->tm_mon   = mo - 1;
  local_now->tm_mday  = 1;
  local_now->tm_isdst = 0;
  time_t local1 = mktime (local_now);
  std::cout << "# local midnight 2013-12-01 " << local1 << '\n';

We can see the local1 date is december 1:

   $ grep local.midnight.2013-12-01 test/all.log
  # local midnight 2013-12-01 1385884800

   $ date -d @1385884800
  Sun Dec  1 00:00:00 PST 2013

Now the following test is performed:

  testParse (t, "2013-W49", 8, year, 0, 49, 0, 0, 0, 0, 0, false, local1);
  ...

  Breakpoint 2, testParse (t=..., input="2013-W49", in_start=8,
    in_year=2013, in_month=0, in_week=49, in_weekday=0, in_julian=0,
    in_day=0, in_seconds=0, in_offset=0, in_utc=false,
    in_date=1385884800)
  at /home/scott/upsrc/libshared/test/datetime.t.cpp:47
  47 {
  (gdb) n
  48        std::string label = std::string ("Datetime::parse (\"") +
                input + "\") --> ";
  (gdb) 
  50        Datetime iso;
  (gdb) 
  51        std::string::size_type start = 0;
  (gdb) 
  53        t.ok (iso.parse (input, start),             label + "true");
  (gdb) 
  ok 147 - Datetime::parse ("2013-W49") --> true
  54        t.is ((int) start,        in_start,         label + "[]");
  (gdb) p iso
  $1 = {static weekstart = 1, static standaloneDateEnabled = true,
    static standaloneTimeEnabled = true, static timeRelative = true,
    _year = 2013, _month = 0, _week = 49, _weekday = 0, _julian = 0,
    _day = 0, _seconds = 0, _offset = 0, _utc = false,
    _date = 1385884800}
  (gdb) n
  ok 148 - Datetime::parse ("2013-W49") --> []
  55        t.is (iso._year,          in_year,          label + "_year");
  (gdb) n
  ok 149 - Datetime::parse ("2013-W49") --> _year
  56        t.is (iso._month,         in_month,         label + "_month");
  (gdb) 
  ok 150 - Datetime::parse ("2013-W49") --> _month
  57        t.is (iso._week,          in_week,          label + "_week");
  (gdb) 
  ok 151 - Datetime::parse ("2013-W49") --> _week

So the test's expectation is that 2013 week 49 is December 1:

   $ task calc 2013-W49
  2013-12-01T00:00:00

Yet, it isn't:

   $ ncal 12 2013
      December 2013
  Mo     2  9 16 23 30
  Tu     3 10 17 24 31
  We     4 11 18 25
  Th     5 12 19 26
  Fr     6 13 20 27
  Sa     7 14 21 28
  Su  1  8 15 22 29
     48 49 50 51 52  1

   $ date -d `task calc 2013-W49` +%V
  48

The week number is wrong, December 1 is still week 48, not week 49. The problem seems to be that it's treating Sunday as the first day of week 49.

Timewarrior does not set or pass weekstart to libshared anywhere, so presumably the libshared default is always used (weekstart = 1, Monday, ISO weeks, as initialized above in Datetime.cpp:62).

Taskwarrior does have an rc.weekstart, but it makes no difference:

   $ for ws in 0 1 Sunday Monday
     do task rc.weekstart=$ws calc 2013-W49
     done
  2013-12-01T00:00:00
  2013-12-01T00:00:00
  2013-12-01T00:00:00
  2013-12-01T00:00:00

This is leading to problems such as in GothenburgBitFactory/timewarrior#595 and GothenburgBitFactory/taskwarrior#2922 with no obvious fix.

I will keep digging into this as time permits. If anyone else knows anything more about this, it would be helpful.

smemsh commented 4 months ago

The problem may be here:

  void Datetime::resolve ()
  {
    ...
    // Convert week + weekday --> julian.
    if (week)
    {
        julian = (week * 7) + weekday - dayOfWeek (year, 1, 4) - 3;
    }
    ...
  }

and:

  // Using Zeller's Congruence.
  // Static
  int Datetime::dayOfWeek (int year, int month, int day)
  {
    int adj = (14 - month) / 12;
    int m = month + 12 * adj - 2;
    int y = year - adj;
    return (day + (13 * m - 1) / 5 + y + y / 4 - y / 100 + y / 400) % 7;
  }

As mentioned in the wikipedia page for Zeller, for ISO weeks there needs to be:

  d = ((h + 5) % 7) + 1

somewhere in there. I'll try to figure it out and make a patch conditioned on weekstart = 1.