mhical rejecting "last <x>day in the month"

Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

mhical rejecting "last <x>day in the month"

Conrad Hughes
mhical rejects things like -1SU (last Sunday in the month), despite its
presence (at a cursory glance) in the ical standard:

  sbr/datetime.c:295:
    if (*cp == '+') { ++cp; } /* +n specific day; don't support '-' */
    else if (*cp == '-') { goto fail; }

.. so RRULEs like "FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU," fail.

Is this just a "not yet implemented", so possibly worth my trying to add?

Conrad

Reply | Threaded
Open this post in threaded view
|

Re: mhical rejecting "last <x>day in the month"

David Levine-3
Conrad writes:

> mhical rejects things like -1SU (last Sunday in the month), despite its
> presence (at a cursory glance) in the ical standard:

That's right.  From the man page:

    mhical supports only a very limited subset of RRULE formats.
    Specifically, only a frequency of YEARLY and an interval of 1 are
    supported.

> Is this just a "not yet implemented", so possibly worth my trying to add?

Sure, that would be great!  The code that parses that YEARLY format
is currently in rrule_clock() in sbr/datetime.c.  Though maybe it
would be better off in the ical parser.

David

Reply | Threaded
Open this post in threaded view
|

Re: mhical rejecting "last <x>day in the month"

Conrad Hughes
Attached is a possible patch for mhical to support "BYDAY=-1SU"-type
RRULEs.  Notes:

  - Added dmlastday() function to dtime.c, to obtain last day of month.
    Minor modifications to other parts here to reuse code.

  - Modified rrule_clock() in datetime.c to handle negative BYDAY
    correctly.

  - Added a couple of tests.

  - I am confused about the tests though: when I run the relevant
    commands by hand, I get a +0100 timezone for the first one, which I
    kindof think I should expect (the event starts during summer time,
    GMT+1); when run as part of the build, I get +0000.  Perhaps someone
    who better understands the test framework could review?

  - Mainly this is to scratch an itch since UK timezone history makes
    heavy use of BYDAY=-1SU-type RRULEs, and the resultant complaints
    are annoying.  I had a brief look at "properly" extending
    icalparse.y, but I kindof think any serious extension of mhical
    should really instead adopt libical?

Conrad


diff --git a/sbr/datetime.c b/sbr/datetime.c
index 333f2dda..29552263 100644
--- a/sbr/datetime.c
+++ b/sbr/datetime.c
@@ -288,14 +288,15 @@ rrule_clock (const char *rrule, const char *starttime, const char *zone,
         int specific_day = 1; /* BYDAY integer (prefix) */
         char buf[32];
         int day;
+        int end_of_week;
 
         if ((cp = nmh_strcasestr (rrule, "BYDAY="))) {
             cp += 6;
             /* BYDAY integers must be ASCII. */
-            if (*cp == '+') { ++cp; } /* +n specific day; don't support '-' */
-            else if (*cp == '-') { goto fail; }
+            if (*cp == '+') { ++cp; } /* +n specific day */
+            else if (*cp == '-') { ++cp; specific_day = -1; }
 
-            if (isdigit ((unsigned char) *cp)) { specific_day = *cp++ - 0x30; }
+            if (isdigit ((unsigned char) *cp)) { specific_day *= *cp++ - 0x30; }
 
             if (! strncasecmp (cp, "SU", 2)) { wday = 0; }
             else if (! strncasecmp (cp, "MO", 2)) { wday = 1; }
@@ -309,12 +310,13 @@ rrule_clock (const char *rrule, const char *starttime, const char *zone,
             month = atoi (cp + 8);
         }
 
-        for (day = 1; day <= 7; ++day) {
+        end_of_week = specific_day > 0 ? 7 * specific_day :
+                dmlastday(year, month) + 7 * (1 + specific_day);
+        for (day = end_of_week - 6; day <= end_of_week; ++day) {
             /* E.g, 11-01-2014 02:00:00-0400 */
             snprintf (buf, sizeof buf, "%02d-%02d-%04u %.2s:%.2s:%.2s%s",
-                      month, day + 7 * (specific_day-1), year,
-                      starttime, starttime + 2, starttime + 4,
-                      zone ? zone : "0000");
+                      month, day, year, starttime, starttime + 2,
+                      starttime + 4, zone ? zone : "0000");
             if ((tws = dparsetime (buf))) {
                 if (! (tws->tw_flags & (TW_SEXP|TW_SIMP))) { set_dotw (tws); }
 
@@ -325,7 +327,7 @@ rrule_clock (const char *rrule, const char *starttime, const char *zone,
             }
         }
 
-        if (day <= 7) {
+        if (day <= end_of_week) {
             clock = tws->tw_clock;
         }
     }
diff --git a/sbr/dtime.c b/sbr/dtime.c
index 9a100265..0ed012b6 100644
--- a/sbr/dtime.c
+++ b/sbr/dtime.c
@@ -20,11 +20,17 @@ extern long timezone;
    2,147,483,648 / 60 = 35,791,394 */
 #define DTZ_BUFFER_SIZE (sizeof "+3579139459")
 
+/*
+ * 1 if leap year, 0 otherwise.
+ */
+#define isleapyear01(y) \
+ (((y) % 4) ? 0 : (((y) % 100) ? 1 : (((y) % 400) ? 0 : 1)))
+
 /*
  * The number of days in the year, accounting for leap years
  */
 #define dysize(y) \
- (((y) % 4) ? 365 : (((y) % 100) ? 366 : (((y) % 400) ? 365 : 366)))
+ (365 + isleapyear01(y))
 
 char *tw_moty[] = {
     "Jan", "Feb", "Mar", "Apr",
@@ -45,8 +51,13 @@ char *tw_ldotw[] = {
     "Saturday",  NULL
 };
 
-static int dmsize[] = {
-    31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
+static int dmsize[][12] = {
+    { /* Not a leap year */
+        31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
+    },
+    { /* Leap year: February = 29 */
+        31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
+    }
 };
 
 
@@ -313,6 +324,16 @@ dtimezone(int offset, int flags)
 }
 
 
+/*
+ * Last day of month (1-12) in given year.
+ */
+
+int
+dmlastday(int year, int mon)
+{
+    return dmsize[isleapyear01 (year)][mon - 1];
+}
+
 /*
  * Convert nmh time structure for local "broken-down"
  * time to calendar time (clock value).  This routine
@@ -323,7 +344,7 @@ dtimezone(int offset, int flags)
 time_t
 dmktime (struct tws *tw)
 {
-    int i, sec, min, hour, mday, mon, year;
+    int i, sec, min, hour, mday, mon, year, *msize;
     time_t result;
 
     if (tw->tw_clock != 0)
@@ -347,10 +368,9 @@ dmktime (struct tws *tw)
 
     for (i = 1970; i < year; i++)
  result += dysize (i);
-    if (dysize (year) == 366 && mon >= 3)
- result++;
+    msize = dmsize[isleapyear01 (year)];
     while (--mon)
- result += dmsize[mon - 1];
+ result += msize[mon - 1];
     result += mday - 1;
     result *= 24; /* Days to hours. */
     result += hour;
diff --git a/sbr/dtime.h b/sbr/dtime.h
index 8accd9ec..24216d40 100644
--- a/sbr/dtime.h
+++ b/sbr/dtime.h
@@ -16,6 +16,7 @@ char *dtimenow(int);
 char *dtime(time_t *, int);
 char *dasctime(struct tws *, int);
 char *dtimezone(int, int);
+int dmlastday(int, int);
 time_t dmktime(struct tws *);
 void set_dotw(struct tws *);
 int twsort(struct tws *, struct tws *);
diff --git a/test/mhical/test-mhical b/test/mhical/test-mhical
index 3ebc35f5..d017a540 100755
--- a/test/mhical/test-mhical
+++ b/test/mhical/test-mhical
@@ -30,6 +30,92 @@ actual="$MH_TEST_DIR/test-mhical$$.actual"
 actual_err="$MH_TEST_DIR/test-mhical$$.actual_err"
 
 
+# check timezone boundary at transition from daylight saving time, -2SU
+start_test "timezone boundary at transition from daylight saving time, -2SU"
+# Specifically looking at "second last Sunday of the month" type transitions.
+cat >"$expected" <<'EOF'
+Summary: BST to GMT
+At: Sat, 22 Oct 1994 23:33 +0000
+To: Sun, 23 Oct 1994 07:34
+EOF
+
+cat >"$MH_TEST_DIR/test1.ics" <<'EOF'
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:test-mhical
+BEGIN:VTIMEZONE
+TZID:London
+BEGIN:STANDARD
+TZNAME:GMT
+DTSTART:19931018T020000
+TZOFFSETFROM:+0100
+TZOFFSETTO:+0000
+RRULE:FREQ=YEARLY;BYDAY=-2SU;BYMONTH=10
+END:STANDARD
+BEGIN:DAYLIGHT
+TZNAME:BST
+DTSTART:19810329T010000
+TZOFFSETFROM:+0000
+TZOFFSETTO:+0100
+RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
+END:DAYLIGHT
+END:VTIMEZONE
+BEGIN:VEVENT
+DTSTAMP:19941002T115852Z
+DTSTART;TZID=London:19941022T233300
+DTEND;TZID=London:19941023T073400
+Summary: BST to GMT
+END:VEVENT
+END:VCALENDAR
+EOF
+
+TZ=GMT mhical <"$MH_TEST_DIR/test1.ics" >"$MH_TEST_DIR/test1.txt"
+check "$expected" "$MH_TEST_DIR/test1.txt"
+
+
+# check timezone boundary at transition to daylight saving time, -1SU
+start_test "timezone boundary at transition to daylight saving time, -1SU"
+# Specifically looking at "last Sunday of the month" type transitions.
+cat >"$expected" <<'EOF'
+Summary: GMT to BST
+At: Sat, 27 Mar 1982 23:31 +0000
+To: Sun, 28 Mar 1982 07:32
+EOF
+
+cat >"$MH_TEST_DIR/test1.ics" <<'EOF'
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:test-mhical
+BEGIN:VTIMEZONE
+TZID:London
+BEGIN:STANDARD
+TZNAME:GMT
+DTSTART:19781025T030000
+TZOFFSETFROM:+0100
+TZOFFSETTO:+0000
+RRULE:FREQ=YEARLY;UNTIL=19811025T010000Z;BYDAY=-1SU;BYMONTH=10
+END:STANDARD
+BEGIN:DAYLIGHT
+TZNAME:BST
+DTSTART:19810329T010000
+TZOFFSETFROM:+0000
+TZOFFSETTO:+0100
+RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
+END:DAYLIGHT
+END:VTIMEZONE
+BEGIN:VEVENT
+DTSTAMP:19820302T115852Z
+DTSTART;TZID=London:19820327T233100
+DTEND;TZID=London:19820328T073200
+Summary: GMT to BST
+END:VEVENT
+END:VCALENDAR
+EOF
+
+TZ=GMT mhical <"$MH_TEST_DIR/test1.ics" >"$MH_TEST_DIR/test1.txt"
+check "$expected" "$MH_TEST_DIR/test1.txt"
+
+
 # check -help
 start_test "-help"
 cat >"$expected" <<EOF
Reply | Threaded
Open this post in threaded view
|

Re: mhical rejecting "last <x>day in the month"

David Levine-3
Hi Conrad,

Wow, thanks!  I'll look at your patch in more detail later.  For now,

  - I am confused about the tests though: when I run the relevant
    commands by hand, I get a +0100 timezone for the first one, which I
    kindof think I should expect (the event starts during summer time,
    GMT+1); when run as part of the build, I get +0000.  Perhaps someone
    who better understands the test framework could review?

mhical obeys TZ.  If not set, behavior is implementation defined.  Some of the tests set TZ, that's the best way to get consistent results.

  - I had a brief look at "properly" extending
    icalparse.y, but I kindof think any serious extension of mhical
    should really instead adopt libical?

Definitely worth considering.  I see that it's subject to two alternative licenses, we'd have to sort through that first.

David 
Reply | Threaded
Open this post in threaded view
|

Re: mhical rejecting "last <x>day in the month"

David Levine-3
I wrote:

> Hi Conrad,
>
> I'll look at your patch in more detail later.

Patch applied.  Thank you.

>       - I am confused about the tests though: when I run the relevant
>         commands by hand, I get a +0100 timezone for the first one, which I
>         kindof think I should expect (the event starts during summer time,
>         GMT+1); when run as part of the build, I get +0000.  Perhaps someone
>         who better understands the test framework could review?

Looking at it again, I think that the test behavior is correct.  Did
you set TZ when you ran mhical by hand?

David

Reply | Threaded
Open this post in threaded view
|

Re: mhical rejecting "last <x>day in the month"

Conrad Hughes
> Looking at it again, I think that the test behavior is correct. Did you
> set TZ when you ran mhical by hand?

I thought I was, but eh, probably doing something silly.  If I see any
actual misbehaviour in the wild I'll follow up.

C.