[PATCH v6 0/9] notmuch search date:since..until query support

Jani Nikula jani at nikula.org
Tue Oct 30 13:32:31 PDT 2012


Hi all, v6 of [1] with plenty of small changes addressing Austin's
review [2], [3], [4], and [5]. See my replies to Austin for what I've
agreed to change, and what I've chosen to ignore and why.

The single biggest change is the requirement to have some delimiter(s)
between keywords, which allowed simplification of keyword
matching. Consequently match_keyword() and parse_keyword() functions in
patch 2/9 have changed considerably.

There are a few ways to examine the changes since v5. My public repo at
[6] has branches topic-parse-time-string-v5 (rebased to master) and
topic-parse-time-string-v6, and [7] should provide a fancy colorful diff
between the two. The same but less fancy diff is also at the end of this
cover letter.

Change by change commits to the parser and test tool can also be found
at [8]. The source files there are copied verbatim to patches 2/9 and
3/9.


BR,
Jani.


[1] id:cover.1350854171.git.jani at nikula.org
[2] id:20121022081444.GM14861 at mit.edu for patch 2/9
[3] id:20121023042326.GP14861 at mit.edu for patch 4/9
[4] id:20121023045255.GQ14861 at mit.edu for patch 6/9
[5] id:20121024210841.GU14861 at mit.edu for patch 8/9
[6] https://gitorious.org/jani/notmuch
[7] https://gitorious.org/jani/notmuch/commit/06c76eb4181bc88eccabc419c690046682125d7a/diffs/ef5e8d111748784433f4b80c9e5378f0c1a57319
[8] https://gitorious.org/parse-time-string/parse-time-string

Jani Nikula (9):
  build: drop the -Wswitch-enum warning
  parse-time-string: add a date/time parser to notmuch
  test: add new test tool parse-time for date/time parser
  test: add smoke tests for the date/time parser module
  build: build parse-time-string as part of the notmuch lib and static
    cli
  lib: add date range query support
  test: add tests for date:since..until range queries
  man: document the date:since..until range queries
  NEWS: date range search support

 Makefile                              |    2 +-
 Makefile.local                        |    2 +-
 NEWS                                  |   12 +
 configure                             |    2 +-
 lib/Makefile.local                    |    3 +-
 lib/database-private.h                |    1 +
 lib/database.cc                       |    5 +
 lib/parse-time-vrp.cc                 |   61 ++
 lib/parse-time-vrp.h                  |   40 +
 man/man7/notmuch-search-terms.7       |  150 +++-
 parse-time-string/Makefile            |    5 +
 parse-time-string/Makefile.local      |   12 +
 parse-time-string/README              |    9 +
 parse-time-string/parse-time-string.c | 1503 +++++++++++++++++++++++++++++++++
 parse-time-string/parse-time-string.h |  102 +++
 test/Makefile.local                   |    7 +-
 test/basic                            |    2 +-
 test/notmuch-test                     |    2 +
 test/parse-time-string                |   78 ++
 test/parse-time.c                     |  314 +++++++
 test/search-date                      |   21 +
 21 files changed, 2315 insertions(+), 18 deletions(-)
 create mode 100644 lib/parse-time-vrp.cc
 create mode 100644 lib/parse-time-vrp.h
 create mode 100644 parse-time-string/Makefile
 create mode 100644 parse-time-string/Makefile.local
 create mode 100644 parse-time-string/README
 create mode 100644 parse-time-string/parse-time-string.c
 create mode 100644 parse-time-string/parse-time-string.h
 create mode 100755 test/parse-time-string
 create mode 100644 test/parse-time.c
 create mode 100755 test/search-date

-- 
1.7.10.4

diff between v5 and v6:

diff --git a/lib/parse-time-vrp.cc b/lib/parse-time-vrp.cc
index 7e4eca4..33f07db 100644
--- a/lib/parse-time-vrp.cc
+++ b/lib/parse-time-vrp.cc
@@ -1,3 +1,24 @@
+/* parse-time-vrp.cc - date range query glue
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2012 Jani Nikula
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Jani Nikula <jani at nikula.org>
+ */
 
 #include "database-private.h"
 #include "parse-time-vrp.h"
diff --git a/lib/parse-time-vrp.h b/lib/parse-time-vrp.h
index 526c217..094c4f8 100644
--- a/lib/parse-time-vrp.h
+++ b/lib/parse-time-vrp.h
@@ -1,3 +1,24 @@
+/* parse-time-vrp.h - date range query glue
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2012 Jani Nikula
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Jani Nikula <jani at nikula.org>
+ */
 
 #ifndef NOTMUCH_PARSE_TIME_VRP_H
 #define NOTMUCH_PARSE_TIME_VRP_H
diff --git a/man/man7/notmuch-search-terms.7 b/man/man7/notmuch-search-terms.7
index fbd3ee7..e39b944 100644
--- a/man/man7/notmuch-search-terms.7
+++ b/man/man7/notmuch-search-terms.7
@@ -141,10 +141,13 @@ expression).
 
 .SH DATE AND TIME SEARCH
 
-This is a non-exhaustive description of the date and time search with
-some pseudo notation. Most of the constructs can be mixed freely, and
-in any order, but the same absolute date or time can't be expressed
-twice.
+notmuch understands a variety of standard and natural ways of
+expressing dates and times, both in absolute terms ("2012-10-24") and
+in relative terms ("yesterday"). Any number of relative terms can be
+combined ("1 hour 25 minutes") and an absolute date/time can be
+combined with relative terms to further adjust it. A non-exhaustive
+description of the syntax supported for absolute and relative terms is
+given below.
 
 .RS 4
 .TP 4
@@ -155,22 +158,22 @@ date:<since>..<until>
 The above expression restricts the results to only messages from
 <since> to <until>, based on the Date: header.
 
-If <since> or <until> describes time at an accuracy of days or less,
-the date/time is rounded, towards past for <since> and towards future
-for <until>, to be inclusive. For example, date:january..february
-matches from the beginning of January until the end of
-February. Similarly, date:yesterday..yesterday matches from the
-beginning of yesterday until the end of yesterday.
+<since> and <until> can describe imprecise times, such as "yesterday".
+In this case, <since> is taken as the earliest time it could describe
+(the beginning of yesterday) and <until> is taken as the latest time
+it could describe (the end of yesterday). Similarly,
+date:january..february matches from the beginning of January to the
+end of February.
+
+Currently, we do not support spaces in range expressions. You can
+replace the spaces with '_', or (in most cases) '-', or (in some
+cases) leave the spaces out altogether. Examples in this man page use
+spaces for clarity.
 
 Open-ended ranges are supported (since Xapian 1.2.1), i.e. it's
 possible to specify date:..<until> or date:<since>.. to not limit the
-start or end time, respectively. Unfortunately, pre-1.2.1 Xapian does
-not report an error on open ended ranges, but it does not work as
-expected either.
-
-Xapian does not support spaces in range expressions. You can replace
-the spaces with '_', or (in most cases) '-', or (in some cases) leave
-the spaces out altogether.
+start or end time, respectively. Pre-1.2.1 Xapian does not report an
+error on open ended ranges, but it does not work as expected either.
 
 Entering date:expr without ".." (for example date:yesterday) won't
 work, as it's not interpreted as a range expression at all. You can
@@ -188,9 +191,9 @@ All refer to past, can be repeated and will be accumulated.
 Units can be abbreviated to any length, with the otherwise ambiguous
 single m being m for minutes and M for months.
 
-Number multiplier can also be written out one, two, ..., ten, dozen,
-hundred. As special cases last means one ("last week") and this means
-zero ("this month").
+Number can also be written out one, two, ..., ten, dozen,
+hundred. Additionally, the unit may be preceded by "last" or "this"
+(e.g., "last week" or "this month").
 
 When combined with absolute date and time, the relative date and time
 specification will be relative from the specified absolute date and
@@ -201,7 +204,7 @@ Examples: 5M2d, two weeks
 
 .RS 4
 .TP 4
-.B Supported time formats
+.B Supported absolute time formats
 H[H]:MM[:SS] [(am|a.m.|pm|p.m.)]
 
 H[H] (am|a.m.|pm|p.m.)
@@ -219,7 +222,7 @@ Examples: 17:05, 5pm
 
 .RS 4
 .TP 4
-.B Supported date formats
+.B Supported absolute date formats
 YYYY-MM[-DD]
 
 DD-MM[-[YY]YY]
diff --git a/parse-time-string/parse-time-string.c b/parse-time-string/parse-time-string.c
index 942041a..584067d3 100644
--- a/parse-time-string/parse-time-string.c
+++ b/parse-time-string/parse-time-string.c
@@ -120,7 +120,7 @@ enum field {
     TM_ABS_MON,		/* month */
     TM_ABS_YEAR,	/* year */
 
-    TM_ABS_WDAY,	/* day of the week. special: may be relative */
+    TM_WDAY,		/* day of the week. special: may be relative */
     TM_ABS_ISDST,	/* daylight saving time */
 
     TM_AMPM,		/* am vs. pm */
@@ -165,9 +165,9 @@ abs_to_rel_field (enum field field)
     return field + (TM_FIRST_REL - TM_FIRST_ABS);
 }
 
-/* Get epoch value for field. */
+/* Get the smallest acceptable value for field. */
 static int
-field_epoch (enum field field)
+get_field_epoch_value (enum field field)
 {
     if (field == TM_ABS_MDAY || field == TM_ABS_MON)
 	return 1;
@@ -208,10 +208,11 @@ get_postponed_length (struct state *state)
  * in fact postponed, false otherwise. Store the postponed number's
  * value in *v, length in the input string in *n (or -1 if the number
  * was written out and parsed as a keyword), and the preceding
- * delimiter to *d.
+ * delimiter to *d. If a number was not postponed, *v, *n and *d are
+ * unchanged.
  */
 static bool
-get_postponed_number (struct state *state, int *v, int *n, char *d)
+consume_postponed_number (struct state *state, int *v, int *n, char *d)
 {
     if (!state->postponed_length)
 	return false;
@@ -279,8 +280,7 @@ is_field_set (struct state *state, enum field field)
 {
     assert (field < ARRAY_SIZE (state->tm));
 
-    return field < ARRAY_SIZE (state->set) &&
-	   state->set[field] != FIELD_UNSET;
+    return state->set[field] != FIELD_UNSET;
 }
 
 static void
@@ -301,10 +301,8 @@ set_field (struct state *state, enum field field, int value)
 {
     int r;
 
-    assert (field < ARRAY_SIZE (state->tm));
-
     /* Fields can only be set once. */
-    if (field < ARRAY_SIZE (state->set) && state->set[field] != FIELD_UNSET)
+    if (is_field_set (state, field))
 	return -PARSE_TIME_ERR_ALREADYSET;
 
     state->set[field] = FIELD_SET;
@@ -347,14 +345,13 @@ set_fields_to_now (struct state *state, enum field *fields, size_t n)
 /* Modify field by adding value to it. To be used on relative fields,
  * which can be modified multiple times (to accumulate). */
 static int
-mod_field (struct state *state, enum field field, int value)
+add_to_field (struct state *state, enum field field, int value)
 {
     int r;
 
-    assert (field < ARRAY_SIZE (state->tm));   /* assert relative??? */
+    assert (field < ARRAY_SIZE (state->tm));
 
-    if (field < ARRAY_SIZE (state->set))
-	state->set[field] = FIELD_SET;
+    state->set[field] = FIELD_SET;
 
     /* Parse a previously postponed number, if any. */
     r = parse_postponed_number (state, field);
@@ -387,7 +384,7 @@ get_field (struct state *state, enum field field)
  */
 static bool is_valid_12hour (int h)
 {
-    return h >= 0 && h <= 12;
+    return h >= 1 && h <= 12;
 }
 
 static bool is_valid_time (int h, int m, int s)
@@ -487,21 +484,15 @@ struct keyword {
  * Setter callback functions for keywords.
  */
 static int
-kw_set_default (struct state *state, struct keyword *kw)
-{
-    return set_field (state, kw->field, kw->value);
-}
-
-static int
 kw_set_rel (struct state *state, struct keyword *kw)
 {
     int multiplier = 1;
 
     /* Get a previously set multiplier, if any. */
-    get_postponed_number (state, &multiplier, NULL, NULL);
+    consume_postponed_number (state, &multiplier, NULL, NULL);
 
     /* Accumulate relative field values. */
-    return mod_field (state, kw->field, multiplier * kw->value);
+    return add_to_field (state, kw->field, multiplier * kw->value);
 }
 
 static int
@@ -521,7 +512,7 @@ kw_set_month (struct state *state, struct keyword *kw)
     if (n == 1 || n == 2) {
 	int r, v;
 
-	get_postponed_number (state, &v, NULL, NULL);
+	consume_postponed_number (state, &v, NULL, NULL);
 
 	if (!is_valid_mday (v))
 	    return -PARSE_TIME_ERR_INVALIDDATE;
@@ -544,7 +535,7 @@ kw_set_ampm (struct state *state, struct keyword *kw)
     if (n == 1 || n == 2) {
 	int r, v;
 
-	get_postponed_number (state, &v, NULL, NULL);
+	consume_postponed_number (state, &v, NULL, NULL);
 
 	if (!is_valid_12hour (v))
 	    return -PARSE_TIME_ERR_INVALIDTIME;
@@ -585,7 +576,7 @@ kw_set_ordinal (struct state *state, struct keyword *kw)
     int n, v;
 
     /* Require a postponed number. */
-    if (!get_postponed_number (state, &v, &n, NULL))
+    if (!consume_postponed_number (state, &v, &n, NULL))
 	return -PARSE_TIME_ERR_DATEFORMAT;
 
     /* Ordinals are mday. */
@@ -605,32 +596,38 @@ kw_set_ordinal (struct state *state, struct keyword *kw)
     return set_field (state, TM_ABS_MDAY, v);
 }
 
+static int
+kw_ignore (unused (struct state *state), unused (struct keyword *kw))
+{
+    return 0;
+}
+
 /*
  * Accepted keywords.
  *
  * A keyword may optionally contain a '|' to indicate the minimum
  * match length. Without one, full match is required. It's advisable
- * to keep the minimum match parts unique across all keywords.
+ * to keep the minimum match parts unique across all keywords. If
+ * they're not, the first match wins.
  *
- * If keyword begins with upper case letter, then the matching will be
- * case sensitive. Otherwise the matching is case insensitive.
+ * If keyword begins with '*', then the matching will be case
+ * sensitive. Otherwise the matching is case insensitive.
  *
- * If setter is NULL, set_default will be used.
+ * If .set is NULL, the field specified by .field will be set to
+ * .value.
  *
- * Note: Order matters. Matching is greedy, longest match is used, but
- * of equal length matches the first one is used, unless there's an
- * equal length case sensitive match which trumps case insensitive
- * matches.
+ * Note: Observe how "m" and "mi" match minutes, "M" and "mo" and
+ * "mont" match months, but "mon" matches Monday.
  */
 static struct keyword keywords[] = {
     /* Weekdays. */
-    { N_("sun|day"),	TM_ABS_WDAY,	0,	NULL },
-    { N_("mon|day"),	TM_ABS_WDAY,	1,	NULL },
-    { N_("tue|sday"),	TM_ABS_WDAY,	2,	NULL },
-    { N_("wed|nesday"),	TM_ABS_WDAY,	3,	NULL },
-    { N_("thu|rsday"),	TM_ABS_WDAY,	4,	NULL },
-    { N_("fri|day"),	TM_ABS_WDAY,	5,	NULL },
-    { N_("sat|urday"),	TM_ABS_WDAY,	6,	NULL },
+    { N_("sun|day"),	TM_WDAY,	0,	NULL },
+    { N_("mon|day"),	TM_WDAY,	1,	NULL },
+    { N_("tue|sday"),	TM_WDAY,	2,	NULL },
+    { N_("wed|nesday"),	TM_WDAY,	3,	NULL },
+    { N_("thu|rsday"),	TM_WDAY,	4,	NULL },
+    { N_("fri|day"),	TM_WDAY,	5,	NULL },
+    { N_("sat|urday"),	TM_WDAY,	6,	NULL },
 
     /* Months. */
     { N_("jan|uary"),	TM_ABS_MON,	1,	kw_set_month },
@@ -648,15 +645,15 @@ static struct keyword keywords[] = {
 
     /* Durations. */
     { N_("y|ears"),	TM_REL_YEAR,	1,	kw_set_rel },
+    { N_("mo|nths"),	TM_REL_MON,	1,	kw_set_rel },
+    { N_("*M"),		TM_REL_MON,	1,	kw_set_rel },
     { N_("w|eeks"),	TM_REL_WEEK,	1,	kw_set_rel },
     { N_("d|ays"),	TM_REL_DAY,	1,	kw_set_rel },
     { N_("h|ours"),	TM_REL_HOUR,	1,	kw_set_rel },
     { N_("hr|s"),	TM_REL_HOUR,	1,	kw_set_rel },
-    { N_("m|inutes"),	TM_REL_MIN,	1,	kw_set_rel },
-    /* M=months, m=minutes */
-    { N_("M"),		TM_REL_MON,	1,	kw_set_rel },
+    { N_("mi|nutes"),	TM_REL_MIN,	1,	kw_set_rel },
     { N_("mins"),	TM_REL_MIN,	1,	kw_set_rel },
-    { N_("mo|nths"),	TM_REL_MON,	1,	kw_set_rel },
+    { N_("*m"),		TM_REL_MIN,	1,	kw_set_rel },
     { N_("s|econds"),	TM_REL_SEC,	1,	kw_set_rel },
     { N_("secs"),	TM_REL_SEC,	1,	kw_set_rel },
 
@@ -692,6 +689,7 @@ static struct keyword keywords[] = {
     { N_("nd"),		TM_NONE,	0,	kw_set_ordinal },
     { N_("rd"),		TM_NONE,	0,	kw_set_ordinal },
     { N_("th"),		TM_NONE,	0,	kw_set_ordinal },
+    { N_("ago"),       	TM_NONE,	0,	kw_ignore },
 
     /* Timezone codes: offset in minutes. XXX: Add more codes. */
     { N_("pst"),	TM_TZ,		-8*60,	NULL },
@@ -715,34 +713,61 @@ static struct keyword keywords[] = {
 };
 
 /*
- * Compare strings s and keyword. Return number of matching chars on
- * match, 0 for no match. Match must be at least n chars, or all of
- * keyword if n < 0, otherwise it's not a match. Use match_case for
- * case sensitive matching.
+ * Compare strings str and keyword. Return the number of matching
+ * chars on match, 0 for no match.
+ *
+ * All of the alphabetic characters (isalpha) in str up to the first
+ * non-alpha character (or end of string) must match the
+ * keyword. Consequently, the value returned on match is the number of
+ * consecutive alphabetic characters in str.
+ *
+ * Abbreviated match is accepted if the keyword contains a '|'
+ * character, and str matches keyword up to that character. Any alpha
+ * characters after that in str must still match the keyword following
+ * the '|' character. If no '|' is present, all of keyword must match.
+ *
+ * Excessive, consecutive, and misplaced (at the beginning or end) '|'
+ * characters in keyword are handled gracefully. Only the first one
+ * matters.
+ *
+ * If match_case is true, the matching is case sensitive.
  */
 static size_t
-match_keyword (const char *s, const char *keyword, ssize_t n, bool match_case)
+match_keyword (const char *str, const char *keyword, bool match_case)
 {
-    ssize_t i;
+    const char *s = str;
+    bool prefix_matched = false;
 
-    if (!n)
-	return 0;
+    for (;;) {
+	while (*keyword == '|') {
+	    prefix_matched = true;
+	    keyword++;
+	}
+
+	if (!*s || !isalpha ((unsigned char) *s) || !*keyword)
+	    break;
 
-    for (i = 0; *s && *keyword; i++, s++, keyword++) {
 	if (match_case) {
 	    if (*s != *keyword)
-		break;
+		return 0;
 	} else {
 	    if (tolower ((unsigned char) *s) !=
 		tolower ((unsigned char) *keyword))
-		break;
+		return 0;
 	}
+	s++;
+	keyword++;
     }
 
-    if (n > 0)
-	return i < n ? 0 : i;
-    else
-	return *keyword ? 0 : i;
+    /* did not match all of the keyword in input string */
+    if (*s && isalpha ((unsigned char) *s))
+	return 0;
+
+    /* did not match enough of keyword */
+    if (*keyword && !prefix_matched)
+	return 0;
+
+    return s - str;
 }
 
 /*
@@ -753,36 +778,24 @@ static ssize_t
 parse_keyword (struct state *state, const char *s)
 {
     unsigned int i;
-    size_t n, max_n = 0;
+    size_t n = 0;
     struct keyword *kw = NULL;
     int r;
 
-    /* Match longest keyword */
     for (i = 0; i < ARRAY_SIZE (keywords); i++) {
-	/* Match case if keyword begins with upper case letter. */
-	bool mcase = isupper ((unsigned char) keywords[i].name[0]);
-	ssize_t minlen = -1;
-	char keyword[128];
-	char *p;
-
-	strncpy (keyword, _(keywords[i].name), sizeof (keyword));
-
-	/* Truncate too long keywords. XXX: Make this dynamic? */
-	keyword[sizeof (keyword) - 1] = '\0';
+	const char *keyword = _(keywords[i].name);
+	bool mcase = false;
 
-	/* Minimum match length. */
-	p = strchr (keyword, '|');
-	if (p) {
-	    minlen = p - keyword;
-
-	    /* Remove the minimum match length separator. */
-	    memmove (p, p + 1, strlen (p + 1) + 1);
+	/* Match case if keyword begins with '*'. */
+	if (*keyword == '*') {
+	    mcase = true;
+	    keyword++;
 	}
 
-	n = match_keyword (s, keyword, minlen, mcase);
-	if (n > max_n || (n == max_n && mcase)) {
-	    max_n = n;
+	n = match_keyword (s, keyword, mcase);
+	if (n) {
 	    kw = &keywords[i];
+	    break;
 	}
     }
 
@@ -792,12 +805,12 @@ parse_keyword (struct state *state, const char *s)
     if (kw->set)
 	r = kw->set (state, kw);
     else
-	r = kw_set_default (state, kw);
+	r = set_field (state, kw->field, kw->value);
 
     if (r < 0)
 	return r;
 
-    return max_n;
+    return n;
 }
 
 /*
@@ -832,7 +845,7 @@ parse_postponed_number (struct state *state, unused (enum field next_field))
     char d;
 
     /* Bail out if there's no postponed number. */
-    if (!get_postponed_number (state, &v, &n, &d))
+    if (!consume_postponed_number (state, &v, &n, &d))
 	return 0;
 
     if (n == 1 || n == 2) {
@@ -884,8 +897,6 @@ parse_postponed_number (struct state *state, unused (enum field next_field))
 	    return -PARSE_TIME_ERR_INVALIDDATE;
 
 	return set_abs_date (state, year, mon, mday);
-    } else {
-	return -PARSE_TIME_ERR_FORMAT;
     }
 
     return -PARSE_TIME_ERR_FORMAT;
@@ -1100,10 +1111,7 @@ parse_number (struct state *state, const char *s)
 
     v1 = strtoul_len (p, &p, &n1);
 
-    if (is_sep (*p) && isdigit ((unsigned char) *(p + 1))) {
-	sep = *p;
-	v2 = strtoul_len (p + 1, &p, &n2);
-    } else {
+    if (!is_sep (*p) || !isdigit ((unsigned char) *(p + 1))) {
 	/* A single number. */
 	r = parse_single_number (state, v1, n1);
 	if (r)
@@ -1112,6 +1120,9 @@ parse_number (struct state *state, const char *s)
 	return p - s;
     }
 
+    sep = *p;
+    v2 = strtoul_len (p + 1, &p, &n2);
+
     /* A group of two or three numbers? */
     if (*p == sep && isdigit ((unsigned char) *(p + 1)))
 	v3 = strtoul_len (p + 1, &p, &n3);
@@ -1199,12 +1210,12 @@ parse_input (struct state *state, const char *s)
  * non-NULL, otherwise current time.
  */
 static int
-initialize_now (struct state *state, struct tm *tm, const time_t *now)
+initialize_now (struct state *state, const time_t *ref, struct tm *tm)
 {
     time_t t;
 
-    if (now) {
-	t = *now;
+    if (ref) {
+	t = *ref;
     } else {
 	if (time (&t) == (time_t) -1)
 	    return -PARSE_TIME_ERR_LIB;
@@ -1229,9 +1240,14 @@ initialize_now (struct state *state, struct tm *tm, const time_t *now)
 }
 
 /*
- * Normalize tm according to mktime(3). Both mktime(3) and
- * localtime_r(3) use local time, but they cancel each other out here,
- * making this function agnostic to time zone.
+ * Normalize tm according to mktime(3); if structure members are
+ * outside their valid interval, they will be normalized (so that, for
+ * example, 40 October is changed into 9 November), and tm_wday and
+ * tm_yday are set to values determined from the contents of the other
+ * fields.
+ *
+ * Both mktime(3) and localtime_r(3) use local time, but they cancel
+ * each other out here, making this function agnostic to time zone.
  */
 static int
 normalize_tm (struct tm *tm)
@@ -1258,7 +1274,7 @@ tm_get_field (const struct tm *tm, enum field field)
     case TM_ABS_MDAY:	return tm->tm_mday;
     case TM_ABS_MON:	return tm->tm_mon + 1; /* 0- to 1-based */
     case TM_ABS_YEAR:	return 1900 + tm->tm_year;
-    case TM_ABS_WDAY:	return tm->tm_wday;
+    case TM_WDAY:	return tm->tm_wday;
     case TM_ABS_ISDST:	return tm->tm_isdst;
     default:
 	assert (false);
@@ -1294,7 +1310,7 @@ fixup_ampm (struct state *state)
 	    hdiff = -12;
     }
 
-    mod_field (state, TM_REL_HOUR, -hdiff);
+    add_to_field (state, TM_REL_HOUR, -hdiff);
 
     return 0;
 }
@@ -1311,7 +1327,7 @@ create_output (struct state *state, time_t *t_out, const time_t *ref,
     int r;
     int week_round = PARSE_TIME_NO_ROUND;
 
-    r = initialize_now (state, &now, ref);
+    r = initialize_now (state, ref, &now);
     if (r)
 	return r;
 
@@ -1330,10 +1346,10 @@ create_output (struct state *state, time_t *t_out, const time_t *ref,
      * months ago wasn't the same day as today. Postpone until we know
      * date?
      */
-    if (is_field_set (state, TM_ABS_WDAY) &&
+    if (is_field_set (state, TM_WDAY) &&
 	!is_field_set (state, TM_ABS_MDAY)) {
-	int wday = get_field (state, TM_ABS_WDAY);
-	int today = tm_get_field (&now, TM_ABS_WDAY);
+	int wday = get_field (state, TM_WDAY);
+	int today = tm_get_field (&now, TM_WDAY);
 	int rel_days;
 
 	if (today > wday)
@@ -1342,9 +1358,9 @@ create_output (struct state *state, time_t *t_out, const time_t *ref,
 	    rel_days = today + 7 - wday;
 
 	/* This also prevents special week rounding from happening. */
-	mod_field (state, TM_REL_DAY, rel_days);
+	add_to_field (state, TM_REL_DAY, rel_days);
 
-	unset_field (state, TM_ABS_WDAY);
+	unset_field (state, TM_WDAY);
     }
 
     r = fixup_ampm (state);
@@ -1361,9 +1377,19 @@ create_output (struct state *state, time_t *t_out, const time_t *ref,
 
 	    if (is_field_set (state, f) || is_field_set (state, r)) {
 		if (round >= PARSE_TIME_ROUND_UP && f != TM_ABS_SEC) {
-		    mod_field (state, r, -1);
+		    /*
+		     * This is the most accurate field
+		     * specified. Round up adjusting it towards
+		     * future.
+		     */
+		    add_to_field (state, r, -1);
+
+		    /*
+		     * Go back a second if the result is to be used
+		     * for inclusive comparisons.
+		     */
 		    if (round == PARSE_TIME_ROUND_UP_INCLUSIVE)
-			mod_field (state, TM_REL_SEC, 1);
+			add_to_field (state, TM_REL_SEC, 1);
 		}
 		round = PARSE_TIME_NO_ROUND; /* No more rounding. */
 	    } else {
@@ -1373,7 +1399,7 @@ create_output (struct state *state, time_t *t_out, const time_t *ref,
 		    week_round = round;
 		    round = PARSE_TIME_NO_ROUND;
 		} else {
-		    set_field (state, f, field_epoch (f));
+		    set_field (state, f, get_field_epoch_value (f));
 		}
 	    }
 	}
diff --git a/test/parse-time-string b/test/parse-time-string
index 862e701..8ae0b4c 100755
--- a/test/parse-time-string
+++ b/test/parse-time-string
@@ -27,19 +27,24 @@ test_begin_subtest "Date parser tests"
 REFERENCE=$(_date Tue Jan 11 11:11:00 +0000 2011)
 cat <<EOF > INPUT
 now          ==> Tue Jan 11 11:11:00 +0000 2011
-2010-1-1     ==> ERROR: 5
+2010-1-1     ==> ERROR: DATEFORMAT
 Jan 2        ==> Sun Jan 02 11:11:00 +0000 2011
 Mon          ==> Mon Jan 10 11:11:00 +0000 2011
-last Friday  ==> ERROR: 4
-2 hours ago  ==> ERROR: 1
+last Friday  ==> ERROR: FORMAT
+2 hours ago  ==> Tue Jan 11 09:11:00 +0000 2011
 last month   ==> Sat Dec 11 11:11:00 +0000 2010
-month ago    ==> ERROR: 1
+month ago    ==> Sat Dec 11 11:11:00 +0000 2010
+two mo       ==> Thu Nov 11 11:11:00 +0000 2010
+3M           ==> Mon Oct 11 11:11:00 +0000 2010
+4-mont       ==> Sat Sep 11 11:11:00 +0000 2010
+5m           ==> Tue Jan 11 11:06:00 +0000 2011
+dozen mi     ==> Tue Jan 11 10:59:00 +0000 2011
 8am          ==> Tue Jan 11 08:00:00 +0000 2011
 9:15         ==> Tue Jan 11 09:15:00 +0000 2011
 12:34        ==> Tue Jan 11 12:34:00 +0000 2011
 monday       ==> Mon Jan 10 11:11:00 +0000 2011
 yesterday    ==> Mon Jan 10 11:11:00 +0000 2011
-tomorrow     ==> ERROR: 1
+tomorrow     ==> ERROR: KEYWORD
              ==> Tue Jan 11 11:11:00 +0000 2011 # empty string is reference time
 
 Aug 3 23:06:06 2012             ==> Fri Aug 03 23:06:06 +0000 2012 # date(1) default format without TZ code
@@ -52,13 +57,15 @@ Fri, 03 Aug 2012 23:07:46 +0100 ==> Fri Aug 03 22:07:46 +0000 2012 # rfc-2822
 
 19701223 +0100 ==> Wed Dec 23 11:11:00 +0000 1970 # Timezone is ignored without an error
 
+today ==^^> Wed Jan 12 00:00:00 +0000 2011
 today ==^> Tue Jan 11 23:59:59 +0000 2011
 today ==_> Tue Jan 11 00:00:00 +0000 2011
 
-thisweek ==^> Sat Jan 15 23:59:59 +0000 2011
-thisweek ==_> Sun Jan 09 00:00:00 +0000 2011
+this week ==^^> Sun Jan 16 00:00:00 +0000 2011
+this week ==^> Sat Jan 15 23:59:59 +0000 2011
+this week ==_> Sun Jan 09 00:00:00 +0000 2011
 
-two months ago==> ERROR: 1 # "ago" is not supported
+two months ago ==> Thu Nov 11 11:11:00 +0000 2010
 two months ==> Thu Nov 11 11:11:00 +0000 2010
 
 @1348569850 ==> Tue Sep 25 10:44:10 +0000 2012
diff --git a/test/parse-time.c b/test/parse-time.c
index 5f73b85..901a4dd 100644
--- a/test/parse-time.c
+++ b/test/parse-time.c
@@ -29,6 +29,28 @@
 
 #define ARRAY_SIZE(a) (sizeof (a) / sizeof (a[0]))
 
+static const char *parse_time_error_strings[] = {
+    [PARSE_TIME_OK]			= "OK",
+    [PARSE_TIME_ERR]			= "ERR",
+    [PARSE_TIME_ERR_LIB]		= "LIB",
+    [PARSE_TIME_ERR_ALREADYSET]		= "ALREADYSET",
+    [PARSE_TIME_ERR_FORMAT]		= "FORMAT",
+    [PARSE_TIME_ERR_DATEFORMAT]		= "DATEFORMAT",
+    [PARSE_TIME_ERR_TIMEFORMAT]		= "TIMEFORMAT",
+    [PARSE_TIME_ERR_INVALIDDATE]	= "INVALIDDATE",
+    [PARSE_TIME_ERR_INVALIDTIME]	= "INVALIDTIME",
+    [PARSE_TIME_ERR_KEYWORD]		= "KEYWORD",
+};
+
+static const char *
+parse_time_strerror (unsigned int errnum)
+{
+    if (errnum < ARRAY_SIZE (parse_time_error_strings))
+	return parse_time_error_strings[errnum];
+    else
+	return NULL;
+}
+
 /*
  * concat argv[start]...argv[end - 1], separating them by a single
  * space, to a malloced string
@@ -188,7 +210,11 @@ parse_stdin (FILE *infile, time_t *ref, int round, const char *format)
 
 	    strftime (result, sizeof (result), format, &tm);
 	} else {
-	    snprintf (result, sizeof (result), "ERROR: %d", r);
+	    const char *errstr = parse_time_strerror (r);
+	    if (errstr)
+		snprintf (result, sizeof (result), "ERROR: %s", errstr);
+	    else
+		snprintf (result, sizeof (result), "ERROR: %d", r);
 	}
 
 	printf ("%s%s %s%s", input, oper, result, trail);
@@ -268,8 +294,15 @@ main (int argc, char *argv[])
 
     free (argstr);
 
-    if (r)
-	return 1;
+    if (r) {
+	const char *errstr = parse_time_strerror (r);
+	if (errstr)
+	    fprintf (stderr, "ERROR: %s\n", errstr);
+	else
+	    fprintf (stderr, "ERROR: %d\n", r);
+
+	return r;
+    }
 
     if (!localtime_r (&result, &tm))
 	return 1;


More information about the notmuch mailing list