Re: [PATCH] getdate add a week when the wday is the same as the current one

[Jim Meyering]

Giuseppe Scrivano wrote:
> I fixed the regression you found and added some new tests.
...
>>From 0e1af7775a82aed00331a535ddadee2753d12e5e Mon Sep 17 00:00:00 2001
> From: Giuseppe Scrivano <address@hidden>
> Date: Fri, 1 May 2009 15:44:30 +0200
> Subject: [PATCH] 
> =?utf-8?q?*=20lib/getdate.y=20(get=5Fdate):=20Correct=20the=20calculation=20of=20tm=5Fmday=20so
> =20that=20e.g.,=20"next=20tues"=20(when=20run=20on=20a=20tuesday)=20results=20in=20a=20date
...
> diff --git a/lib/getdate.y b/lib/getdate.y
...
> @@ -1435,7 +1435,9 @@ get_date (struct timespec *result, char const *p, 
> struct timespec const *now)
>        if (pc.days_seen && ! pc.dates_seen)
>       {
>         tm.tm_mday += ((pc.day_number - tm.tm_wday + 7) % 7
> -                      + 7 * (pc.day_ordinal - (0 < pc.day_ordinal)));
> +                      + 7 * (pc.day_ordinal
> +                          - (0 < pc.day_ordinal
> +                             && tm.tm_wday != pc.day_number)));

Thank you!
I've just pushed the following two change-sets.
The first is yours modulo log syntax and spaces vs. TABs,
then one that tightens the tests to require exact values, not just ">".

>From 9b518aa03eb0ea04dac464d0bb4db6377d84cf79 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Fri, 1 May 2009 09:23:20 +0200
Subject: [PATCH 1/2] getdate: correctly interpret "next monday" when run on a 
Monday
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

* lib/getdate.y (get_date): Correct the calculation of tm_mday so
that e.g., "next tues" (when run on a tuesday) results in a date
that is one week in the future, and not today's date.
I.e., add a week when the wday is the same as the current one.
Reported by Tom Broadhurst in http://savannah.gnu.org/bugs/?25406,
and earlier by Martin Bernreuther and Jan Minář.
* tests/test-getdate.c (main): Check that "next DAY" is always in
the future and that "last DAY" is always in the past.
---
 ChangeLog            |   12 +++++++++++
 lib/getdate.y        |   10 +++++---
 tests/test-getdate.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3d72119..15bc793 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-05-03  Giuseppe Scrivano  <address@hidden>
+
+       getdate: correctly interpret "next monday" when run on a Monday
+       * lib/getdate.y (get_date): Correct the calculation of tm_mday so
+       that e.g., "next tues" (when run on a tuesday) results in a date
+       that is one week in the future, and not today's date.
+       I.e., add a week when the wday is the same as the current one.
+       Reported by Tom Broadhurst in http://savannah.gnu.org/bugs/?25406,
+       and earlier by Martin Bernreuther and Jan Minář.
+       * tests/test-getdate.c (main): Check that "next DAY" is always in
+       the future and that "last DAY" is always in the past.
+
 2009-05-02  Jim Meyering  <address@hidden>

        build: ensure that a release build fails when a submodule is unclean
diff --git a/lib/getdate.y b/lib/getdate.y
index 877b264..2f3c924 100644
--- a/lib/getdate.y
+++ b/lib/getdate.y
@@ -1,7 +1,7 @@
 %{
 /* Parse a string into an internal time stamp.

-   Copyright (C) 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007, 2008
+   Copyright (C) 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
    Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
@@ -404,12 +404,12 @@ zone:
 day:
     tDAY
       {
-       pc->day_ordinal = 1;
+       pc->day_ordinal = 0;
        pc->day_number = $1;
       }
   | tDAY ','
       {
-       pc->day_ordinal = 1;
+       pc->day_ordinal = 0;
        pc->day_number = $1;
       }
   | tORDINAL tDAY
@@ -1435,7 +1435,9 @@ get_date (struct timespec *result, char const *p, struct 
timespec const *now)
       if (pc.days_seen && ! pc.dates_seen)
        {
          tm.tm_mday += ((pc.day_number - tm.tm_wday + 7) % 7
-                        + 7 * (pc.day_ordinal - (0 < pc.day_ordinal)));
+                        + 7 * (pc.day_ordinal
+                               - (0 < pc.day_ordinal
+                                  && tm.tm_wday != pc.day_number)));
          tm.tm_isdst = -1;
          Start = mktime (&tm);
          if (Start == (time_t) -1)
diff --git a/tests/test-getdate.c b/tests/test-getdate.c
index 7dfb09e..73c3ad1 100644
--- a/tests/test-getdate.c
+++ b/tests/test-getdate.c
@@ -48,6 +48,22 @@
 #define LOG(str, now, res) (void) 0
 #endif

+static const char* const day_table[] =
+{
+  "SUNDAY",
+  "MONDAY",
+  "TUESDAY",
+  "TUES",
+  "WEDNESDAY",
+  "WEDNES",
+  "THURSDAY",
+  "THUR",
+  "THURS",
+  "FRIDAY",
+  "SATURDAY",
+  NULL
+};
+
 int
 main (int argc, char **argv)
 {
@@ -55,6 +71,7 @@ main (int argc, char **argv)
   struct timespec result2;
   struct timespec now;
   const char *p;
+  int i;

   set_program_name (argv[0]);

@@ -211,5 +228,42 @@ main (int argc, char **argv)
   ASSERT (result.tv_sec == result2.tv_sec
          && result.tv_nsec == result2.tv_nsec);

+  /* Check that every 'last/next DAY' is in the past/future.  */
+  for (i = 0; day_table[i]; i++)
+    {
+      char tmp[32];
+      sprintf (tmp, "NEXT %s", day_table[i]);
+      now.tv_sec = 4711;
+      now.tv_nsec = 1267;
+      ASSERT (get_date (&result, tmp, &now));
+      LOG (tmp, now, result);
+      ASSERT (result.tv_sec > now.tv_sec
+              && result.tv_nsec == 0);
+
+      sprintf (tmp, "LAST %s", day_table[i]);
+      now.tv_sec = 4711;
+      now.tv_nsec = 1267;
+      ASSERT (get_date (&result, tmp, &now));
+      LOG (tmp, now, result);
+      ASSERT (result.tv_sec < now.tv_sec
+              && result.tv_nsec == 0);
+    }
+
+  p = "THURSDAY UTC+00";  /* The epoch was on Thursday.  */
+  now.tv_sec = 0;
+  now.tv_nsec = 0;
+  ASSERT (get_date (&result, p, &now));
+  LOG (p, now, result);
+  ASSERT (result.tv_sec == now.tv_sec
+         && result.tv_nsec == now.tv_nsec);
+
+  p = "FRIDAY UTC+00";
+  now.tv_sec = 0;
+  now.tv_nsec = 0;
+  ASSERT (get_date (&result, p, &now));
+  LOG (p, now, result);
+  ASSERT (result.tv_sec >= now.tv_sec
+         && result.tv_nsec == now.tv_nsec);
+
   return 0;
 }
--
1.6.3.rc4.190.g4648


>From 8d08fa04b902a9fdd504c6ceae2efb8ca650c2a6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 3 May 2009 09:17:53 +0200
Subject: [PATCH 2/2] tests: tighten some getdate tests

* tests/test-getdate.c (main): Tighten tests: require equality,
not just greater than.  Set TZ envvar to UTC0.
---
 ChangeLog            |    6 ++++++
 tests/test-getdate.c |   22 ++++++++++------------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 15bc793..817788a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-05-03  Jim Meyering  <address@hidden>
+
+       tests: tighten some getdate tests
+       * tests/test-getdate.c (main): Tighten tests: require equality,
+       not just greater than.  Set TZ envvar to UTC0.
+
 2009-05-03  Giuseppe Scrivano  <address@hidden>

        getdate: correctly interpret "next monday" when run on a Monday
diff --git a/tests/test-getdate.c b/tests/test-getdate.c
index 73c3ad1..ea70527 100644
--- a/tests/test-getdate.c
+++ b/tests/test-getdate.c
@@ -53,12 +53,8 @@ static const char* const day_table[] =
   "SUNDAY",
   "MONDAY",
   "TUESDAY",
-  "TUES",
   "WEDNESDAY",
-  "WEDNES",
   "THURSDAY",
-  "THUR",
-  "THURS",
   "FRIDAY",
   "SATURDAY",
   NULL
@@ -228,25 +224,27 @@ main (int argc, char **argv)
   ASSERT (result.tv_sec == result2.tv_sec
          && result.tv_nsec == result2.tv_nsec);

-  /* Check that every 'last/next DAY' is in the past/future.  */
+  /* Check that some "next Monday", "last Wednesday", etc. are correct.  */
+  putenv ("TZ=UTC0");
   for (i = 0; day_table[i]; i++)
     {
+      unsigned int thur2 = 7 * 24 * 3600; /* 2nd thursday */
       char tmp[32];
       sprintf (tmp, "NEXT %s", day_table[i]);
-      now.tv_sec = 4711;
+      now.tv_sec = thur2 + 4711;
       now.tv_nsec = 1267;
       ASSERT (get_date (&result, tmp, &now));
       LOG (tmp, now, result);
-      ASSERT (result.tv_sec > now.tv_sec
-              && result.tv_nsec == 0);
+      ASSERT (result.tv_nsec == 0);
+      ASSERT (result.tv_sec == thur2 + (i == 4 ? 7 : (i + 3) % 7) * 24 * 3600);

       sprintf (tmp, "LAST %s", day_table[i]);
-      now.tv_sec = 4711;
+      now.tv_sec = thur2 + 4711;
       now.tv_nsec = 1267;
       ASSERT (get_date (&result, tmp, &now));
       LOG (tmp, now, result);
-      ASSERT (result.tv_sec < now.tv_sec
-              && result.tv_nsec == 0);
+      ASSERT (result.tv_nsec == 0);
+      ASSERT (result.tv_sec == thur2 + ((i + 3) % 7 - 7) * 24 * 3600);
     }

   p = "THURSDAY UTC+00";  /* The epoch was on Thursday.  */
@@ -262,7 +260,7 @@ main (int argc, char **argv)
   now.tv_nsec = 0;
   ASSERT (get_date (&result, p, &now));
   LOG (p, now, result);
-  ASSERT (result.tv_sec >= now.tv_sec
+  ASSERT (result.tv_sec == 24 * 3600
          && result.tv_nsec == now.tv_nsec);

   return 0;
--
1.6.3.rc4.190.g4648