{"thread":{"id":"20700","subject":"Improve on 'approxidate'","startedAt":"2009-08-22T22:10:07Z","lastAt":"2009-09-01T03:27:03Z","messageCount":6,"participants":["Linus Torvalds","Nicolas Pitre","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"121529","messageId":"alpine.LFD.2.01.0908221438450.3158@localhost.localdomain","threadId":"20700","inReplyTo":null,"subject":"Improve on 'approxidate'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-08-22T22:10:07Z","receivedAt":"2009-08-22T22:10:07Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis is not a new failure mode - approxidate has always been kind of \nrandom in the input it accepts, but some of the randomness is more \nirritating than others.\n\nFor example:\n\n\tJun 6, 5AM -> Mon Jun 22 05:00:00 2009\n\t5AM Jun 6 -> Sat Jun  6 05:00:00 2009\n\nWhaa? The reason for the above is that approxidate squirrells away the '6' \nfrom \"Jun 6\" to see if it's going to be a relative number, and then \nforgets about it when it sees a new number (the '5' in '5AM'). So the odd \n\"June 22\" date is because today is July 22nd, and if it doesn't have \nanother day of the month, it will just pick todays mday - having ignored \nthe '6' entirely due to getting all excited about seeing a new number (5).\n\nThere are other oddnesses. This does not fix them all, but I think it \nmakes for fewer _really_ perplexing cases. At least now we have\n\n\tJun 6, 5AM -> Sat Jun  6 05:00:00 2009\n\t5AM, Jun 6 -> Sat Jun  6 05:00:00 2009\n\nwhich makes me happier. I can still point to cases that don't work as \nwell, but those are separate issues.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n date.c |   93 +++++++++++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 63 insertions(+), 30 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 409a17d..51c6461 100644\n--- a/date.c\n+++ b/date.c\n@@ -525,11 +525,8 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\t}\n \t}\n \n-\tif (num > 0 && num < 32) {\n-\t\ttm->tm_mday = num;\n-\t} else if (num > 0 && num < 13) {\n+\tif (num > 0 && num < 13 && tm->tm_mon < 0)\n \t\ttm->tm_mon = num-1;\n-\t}\n \n \treturn n;\n }\n@@ -657,42 +654,59 @@ void datestamp(char *buf, int bufsize)\n \tdate_string(now, offset, buf, bufsize);\n }\n \n-static void update_tm(struct tm *tm, unsigned long sec)\n+/*\n+ * Relative time update (eg \"2 days ago\").  If we haven't set the time\n+ * yet, we need to set it from current time.\n+ */\n+static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)\n {\n-\ttime_t n = mktime(tm) - sec;\n+\ttime_t n;\n+\n+\tif (tm->tm_mday < 0)\n+\t\ttm->tm_mday = now->tm_mday;\n+\tif (tm->tm_mon < 0)\n+\t\ttm->tm_mon = now->tm_mon;\n+\tif (tm->tm_year < 0) {\n+\t\ttm->tm_year = now->tm_year;\n+\t\tif (tm->tm_mon > now->tm_mon)\n+\t\t\ttm->tm_year--;\n+\t}\n+\n+\tn = mktime(tm) - sec;\n \tlocaltime_r(&n, tm);\n+\treturn n;\n }\n \n-static void date_yesterday(struct tm *tm, int *num)\n+static void date_yesterday(struct tm *tm, struct tm *now, int *num)\n {\n-\tupdate_tm(tm, 24*60*60);\n+\tupdate_tm(tm, now, 24*60*60);\n }\n \n-static void date_time(struct tm *tm, int hour)\n+static void date_time(struct tm *tm, struct tm *now, int hour)\n {\n \tif (tm->tm_hour < hour)\n-\t\tdate_yesterday(tm, NULL);\n+\t\tdate_yesterday(tm, now, NULL);\n \ttm->tm_hour = hour;\n \ttm->tm_min = 0;\n \ttm->tm_sec = 0;\n }\n \n-static void date_midnight(struct tm *tm, int *num)\n+static void date_midnight(struct tm *tm, struct tm *now, int *num)\n {\n-\tdate_time(tm, 0);\n+\tdate_time(tm, now, 0);\n }\n \n-static void date_noon(struct tm *tm, int *num)\n+static void date_noon(struct tm *tm, struct tm *now, int *num)\n {\n-\tdate_time(tm, 12);\n+\tdate_time(tm, now, 12);\n }\n \n-static void date_tea(struct tm *tm, int *num)\n+static void date_tea(struct tm *tm, struct tm *now, int *num)\n {\n-\tdate_time(tm, 17);\n+\tdate_time(tm, now, 17);\n }\n \n-static void date_pm(struct tm *tm, int *num)\n+static void date_pm(struct tm *tm, struct tm *now, int *num)\n {\n \tint hour, n = *num;\n \t*num = 0;\n@@ -706,7 +720,7 @@ static void date_pm(struct tm *tm, int *num)\n \ttm->tm_hour = (hour % 12) + 12;\n }\n \n-static void date_am(struct tm *tm, int *num)\n+static void date_am(struct tm *tm, struct tm *now, int *num)\n {\n \tint hour, n = *num;\n \t*num = 0;\n@@ -720,7 +734,7 @@ static void date_am(struct tm *tm, int *num)\n \ttm->tm_hour = (hour % 12);\n }\n \n-static void date_never(struct tm *tm, int *num)\n+static void date_never(struct tm *tm, struct tm *now, int *num)\n {\n \ttime_t n = 0;\n \tlocaltime_r(&n, tm);\n@@ -728,7 +742,7 @@ static void date_never(struct tm *tm, int *num)\n \n static const struct special {\n \tconst char *name;\n-\tvoid (*fn)(struct tm *, int *);\n+\tvoid (*fn)(struct tm *, struct tm *, int *);\n } special[] = {\n \t{ \"yesterday\", date_yesterday },\n \t{ \"noon\", date_noon },\n@@ -757,7 +771,7 @@ static const struct typelen {\n \t{ NULL }\n };\n \n-static const char *approxidate_alpha(const char *date, struct tm *tm, int *num)\n+static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm *now, int *num)\n {\n \tconst struct typelen *tl;\n \tconst struct special *s;\n@@ -778,7 +792,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, int *num)\n \tfor (s = special; s->name; s++) {\n \t\tint len = strlen(s->name);\n \t\tif (match_string(date, s->name) == len) {\n-\t\t\ts->fn(tm, num);\n+\t\t\ts->fn(tm, now, num);\n \t\t\treturn end;\n \t\t}\n \t}\n@@ -800,7 +814,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, int *num)\n \twhile (tl->type) {\n \t\tint len = strlen(tl->type);\n \t\tif (match_string(date, tl->type) >= len-1) {\n-\t\t\tupdate_tm(tm, tl->length * *num);\n+\t\t\tupdate_tm(tm, now, tl->length * *num);\n \t\t\t*num = 0;\n \t\t\treturn end;\n \t\t}\n@@ -818,7 +832,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, int *num)\n \t\t\t\tn++;\n \t\t\tdiff += 7*n;\n \n-\t\t\tupdate_tm(tm, diff * 24 * 60 * 60);\n+\t\t\tupdate_tm(tm, now, diff * 24 * 60 * 60);\n \t\t\treturn end;\n \t\t}\n \t}\n@@ -866,6 +880,22 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num)\n \treturn end;\n }\n \n+/*\n+ * Do we have a pending number at the end, or when\n+ * we see a new one? Let's assume it's a month day,\n+ * as in \"Dec 6, 1992\"\n+ */\n+static void pending_number(struct tm *tm, int *num)\n+{\n+\tint number = *num;\n+\n+\tif (number) {\n+\t\t*num = 0;\n+\t\tif (tm->tm_mday < 0 && number < 32)\n+\t\t\ttm->tm_mday = number;\n+\t}\n+}\n+\n unsigned long approxidate(const char *date)\n {\n \tint number = 0;\n@@ -881,21 +911,24 @@ unsigned long approxidate(const char *date)\n \ttime_sec = tv.tv_sec;\n \tlocaltime_r(&time_sec, &tm);\n \tnow = tm;\n+\n+\ttm.tm_year = -1;\n+\ttm.tm_mon = -1;\n+\ttm.tm_mday = -1;\n+\n \tfor (;;) {\n \t\tunsigned char c = *date;\n \t\tif (!c)\n \t\t\tbreak;\n \t\tdate++;\n \t\tif (isdigit(c)) {\n+\t\t\tpending_number(&tm, &number);\n \t\t\tdate = approxidate_digit(date-1, &tm, &number);\n \t\t\tcontinue;\n \t\t}\n \t\tif (isalpha(c))\n-\t\t\tdate = approxidate_alpha(date-1, &tm, &number);\n+\t\t\tdate = approxidate_alpha(date-1, &tm, &now, &number);\n \t}\n-\tif (number > 0 && number < 32)\n-\t\ttm.tm_mday = number;\n-\tif (tm.tm_mon > now.tm_mon && tm.tm_year == now.tm_year)\n-\t\ttm.tm_year--;\n-\treturn mktime(&tm);\n+\tpending_number(&tm, &number);\n+\treturn update_tm(&tm, &now, 0);\n }\n"},{"id":"121534","messageId":"alpine.LFD.2.01.0908221759420.3158@localhost.localdomain","threadId":"20700","inReplyTo":"alpine.LFD.2.01.0908221438450.3158@localhost.localdomain","subject":"Further 'approxidate' improvements","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-08-23T01:11:44Z","receivedAt":"2009-08-23T01:11:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThe previous patch to improve approxidate got us to the point that a lot \nof the remaining annoyances were due to the 'strict' date handling running \nfirst, and deciding that it got a good enough date that the approximate \ndate routines were never even invoked.\n\nFor example, using a date string like\n\n\t6AM, June 7, 2009\n\nthe strict date logic would be perfectly happy with the \"June 7, 2009\" \npart, and ignore the 6AM part that it didn't understand - resulting in the \ninformation getting dropped on the floor:\n\n\t6AM, June 7, 2009 -> Sat Jun 6 00:00:00 2009\n\nand the date being calculated as if it was midnight, and the '6AM' having \nconfused the date routines into thinking about '6 June' rather than 'June \n7' at 6AM (ie notice how the _day_ was wrong due to this, not just the \ntime).\n\nSo this makes the strict date routines a bit stricter, and requires that \nnot just the date, but also the time, has actually been parsed. With that \nfix, and trivial extension of the approxidate routines, git now properly \nparses the date as\n\n\t6AM, June 7, 2009 -> Sun Jun  7 06:00:00 2009\n\nwithout dropping the fuzzy time (\"6AM\" or \"noon\" or any of the other \nnon-strict time formats) on the floor.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\nOn Sat, 22 Aug 2009, Linus Torvalds wrote:\n> \n> There are other oddnesses. This does not fix them all, but I think it \n> makes for fewer _really_ perplexing cases. At least now we have\n> \n> \tJun 6, 5AM -> Sat Jun  6 05:00:00 2009\n> \t5AM, Jun 6 -> Sat Jun  6 05:00:00 2009\n> \n> which makes me happier. I can still point to cases that don't work as \n> well, but those are separate issues.\n\nThis gets rid of the remaining \"obviously bogus\" issues with parsing of \nfuzzy dates. I'm sure there are other issues still remaining, but now I \ncan't come up with any trivial cases any more without having clear \ngarbage in the string. \n\nSo trying to date-parse nonsensical crud still gives odd results:\n\n\tI ate six hot-dogs in June -> Sat Jun  6 18:09:26 2009\n\nbecause it parses \"six\" and \"June\" and then puts it together as a date, \nand then adds the current time (and year) and is happy.\n\nBut parsing random things amusingly is a _feature_. Misparsing something \nthat makes sense as a date is a bug.\n\n date.c |   32 +++++++++++++++++++++++++++-----\n 1 files changed, 27 insertions(+), 5 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 51c6461..1de1845 100644\n--- a/date.c\n+++ b/date.c\n@@ -24,6 +24,8 @@ time_t tm_to_time_t(const struct tm *tm)\n \t\treturn -1;\n \tif (month < 2 || (year + 2) % 4)\n \t\tday--;\n+\tif (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_sec < 0)\n+\t\treturn -1;\n \treturn (year * 365 + (year + 1) / 4 + mdays[month] + day) * 24*60*60UL +\n \t\ttm->tm_hour * 60*60 + tm->tm_min * 60 + tm->tm_sec;\n }\n@@ -425,13 +427,19 @@ static int match_multi_number(unsigned long num, char c, const char *date, char\n \treturn end - date;\n }\n \n-/* Have we filled in any part of the time/date yet? */\n+/*\n+ * Have we filled in any part of the time/date yet?\n+ * We just do a binary 'and' to see if the sign bit\n+ * is set in all the values.\n+ */\n static inline int nodate(struct tm *tm)\n {\n-\treturn tm->tm_year < 0 &&\n-\t\ttm->tm_mon < 0 &&\n-\t\ttm->tm_mday < 0 &&\n-\t\t!(tm->tm_hour | tm->tm_min | tm->tm_sec);\n+\treturn (tm->tm_year &\n+\t\ttm->tm_mon &\n+\t\ttm->tm_mday &\n+\t\ttm->tm_hour &\n+\t\ttm->tm_min &\n+\t\ttm->tm_sec) < 0;\n }\n \n /*\n@@ -580,6 +588,9 @@ int parse_date(const char *date, char *result, int maxlen)\n \ttm.tm_mon = -1;\n \ttm.tm_mday = -1;\n \ttm.tm_isdst = -1;\n+\ttm.tm_hour = -1;\n+\ttm.tm_min = -1;\n+\ttm.tm_sec = -1;\n \toffset = -1;\n \ttm_gmt = 0;\n \n@@ -893,6 +904,17 @@ static void pending_number(struct tm *tm, int *num)\n \t\t*num = 0;\n \t\tif (tm->tm_mday < 0 && number < 32)\n \t\t\ttm->tm_mday = number;\n+\t\telse if (tm->tm_mon < 0 && number < 13)\n+\t\t\ttm->tm_mon = number-1;\n+\t\telse if (tm->tm_year < 0) {\n+\t\t\tif (number > 1969 && number < 2100)\n+\t\t\t\ttm->tm_year = number - 1900;\n+\t\t\telse if (number > 69 && number < 100)\n+\t\t\t\ttm->tm_year = number;\n+\t\t\telse if (number < 38)\n+\t\t\t\ttm->tm_year = 100 + number;\n+\t\t\t/* We screw up for number = 00 ? */\n+\t\t}\n \t}\n }\n \n"},{"id":"121535","messageId":"alpine.LFD.2.00.0908222204590.6044@xanadu.home","threadId":"20700","inReplyTo":"alpine.LFD.2.01.0908221759420.3158@localhost.localdomain","subject":"Re: Further 'approxidate' improvements","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-08-23T02:08:17Z","receivedAt":"2009-08-23T02:08:17Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 22 Aug 2009, Linus Torvalds wrote:\n\n> So trying to date-parse nonsensical crud still gives odd results:\n> \n> \tI ate six hot-dogs in June -> Sat Jun  6 18:09:26 2009\n> \n> because it parses \"six\" and \"June\" and then puts it together as a date, \n> and then adds the current time (and year) and is happy.\n> \n> But parsing random things amusingly is a _feature_. Misparsing something \n> that makes sense as a date is a bug.\n\nMaybe that would be a good idea to write a test just for this, so known \ncases making sense aren't accidentally broken by eventual modifications \nto add more such cases.\n\n\nNicolas\n"},{"id":"122147","messageId":"20090830223558.GA29807@coredump.intra.peff.net","threadId":"20700","inReplyTo":"alpine.LFD.2.01.0908221438450.3158@localhost.localdomain","subject":"Re: Improve on 'approxidate'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-30T22:35:58Z","receivedAt":"2009-08-30T22:35:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 22, 2009 at 03:10:07PM -0700, Linus Torvalds wrote:\n\n>  unsigned long approxidate(const char *date)\n>  {\n>  \tint number = 0;\n> @@ -881,21 +911,24 @@ unsigned long approxidate(const char *date)\n>  \ttime_sec = tv.tv_sec;\n>  \tlocaltime_r(&time_sec, &tm);\n>  \tnow = tm;\n> +\n> +\ttm.tm_year = -1;\n> +\ttm.tm_mon = -1;\n> +\ttm.tm_mday = -1;\n\nThis breaks relative dates like \"3.months.ago\", because\napproxidate_alpha needs to see the \"current\" date in tm (and now it sees\n-1, subtracts from it, and assumes we are just crossing a year boundary\nbecause of the negative).  3.years.ago is also broken, but I don't think\n3.days.ago is.\n\nProbably we just need to pass \"now\" to approxidate_alpha, and it needs\nto call update_tm under the case for \"months\" and \"years\" (and I haven't\nquite figured out why those are not part of the \"tl\" list).\nUnfortunately, I'm out of time to look at it more right now, but I'll\ntake a look tonight or tomorrow if you don't beat me to it.\n\n-Peff\n"},{"id":"122154","messageId":"20090831015817.GA30454@coredump.intra.peff.net","threadId":"20700","inReplyTo":"20090830223558.GA29807@coredump.intra.peff.net","subject":"Re: Improve on 'approxidate'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T01:58:17Z","receivedAt":"2009-08-31T01:58:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 30, 2009 at 06:35:58PM -0400, Jeff King wrote:\n\n> > +\ttm.tm_year = -1;\n> > +\ttm.tm_mon = -1;\n> > +\ttm.tm_mday = -1;\n> \n> This breaks relative dates like \"3.months.ago\", because\n> approxidate_alpha needs to see the \"current\" date in tm (and now it sees\n> -1, subtracts from it, and assumes we are just crossing a year boundary\n> because of the negative).  3.years.ago is also broken, but I don't think\n> 3.days.ago is.\n> \n> Probably we just need to pass \"now\" to approxidate_alpha, and it needs\n> to call update_tm under the case for \"months\" and \"years\" (and I haven't\n> quite figured out why those are not part of the \"tl\" list).\n> Unfortunately, I'm out of time to look at it more right now, but I'll\n> take a look tonight or tomorrow if you don't beat me to it.\n\nOK, I looked at it.\n\nThe fix is pretty straightforward. We _do_ already pass \"now\" to\napproxidate_alpha, and it looks like you already fixed the \"typelen\"\narray case (which handles seconds, minutes, hours, days, and weeks) by\ncalling update_tm. But all of those units are convertible to seconds,\nand months and years are not, which explains why they are handled\nseparately.\n\nSo I think we can just \"cheat\" and call update_tm to fill in the fields\nfrom \"now\" as we would for the other units, and then tweak the \"struct\ntm\" as we did before. I.e.,:\n\ndiff --git a/date.c b/date.c\nindex 8e57e5e..e9ee4aa 100644\n--- a/date.c\n+++ b/date.c\n@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n \t}\n \n \tif (match_string(date, \"months\") >= 5) {\n-\t\tint n = tm->tm_mon - *num;\n+\t\tint n;\n+\t\tupdate_tm(tm, now, 0); /* fill in date fields if needed */\n+\t\tn = tm->tm_mon - *num;\n \t\t*num = 0;\n \t\twhile (n < 0) {\n \t\t\tn += 12;\n@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n \t}\n \n \tif (match_string(date, \"years\") >= 4) {\n+\t\tupdate_tm(tm, now, 0); /* fill in date fields if needed */\n \t\ttm->tm_year -= *num;\n \t\t*num = 0;\n \t\treturn end;\n\nI'll wrap this fix up in a commit message with tests and add it to the\n\"test approxidate\" series I'm brewing.\n\n-Peff\n"},{"id":"122222","messageId":"alpine.LFD.2.00.0908311725260.21913@eeepc.linux-foundation.org","threadId":"20700","inReplyTo":"20090830223558.GA29807@coredump.intra.peff.net","subject":"Re: Improve on 'approxidate'","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-09-01T03:27:03Z","receivedAt":"2009-09-01T03:27:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 30 Aug 2009, Jeff King wrote:\n> \n> This breaks relative dates like \"3.months.ago\", because\n> approxidate_alpha needs to see the \"current\" date in tm (and now it sees\n> -1, subtracts from it, and assumes we are just crossing a year boundary\n> because of the negative).  3.years.ago is also broken, but I don't think\n> 3.days.ago is.\n\nGaah. Thanks for noticing and the fixes. I had tested the relative modes, \nbut only the \"fixed offset\" ones (days, hours, minutes, seconds), not the \nmonths and years cases.\n\n\t\t\tLinus\n"}]}