Re: [PATCH 2/2] approxidate: allow ISO-like dates far in the future

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 13, 2014 at 04:36:47PM -0500, Jeff King wrote:

> On Thu, Nov 13, 2014 at 01:11:46PM -0800, Junio C Hamano wrote:
> 
> > Jeff King <peff@xxxxxxxx> writes:
> > 
> > >  		if (c != '.' &&
> > > -		    is_date(num3, num, num2, refuse_future, now, tm))
> > > +		    is_date(num3, num, num2, refuse_future, now, tm, 0))
> > >  			break;
> > 
> > Doesn't the new argument '0', which is "allow-future", look somewhat
> > strange when we are already passing refuse_future?
> 
> To be honest, I had trouble figuring out what the name "refuse_future"
> really meant. We do skip the future check, but it also means that
> is_date will munge the "struct tm" directly, even if we do not find a
> valid date. That worried me a bit.
> 
> But yeah, in theory, the callers I wanted to tweak can just pass in a
> NULL refuse_future.

So here's what the patch looks like just using refuse_future.

It's definitely nicer to read, and it passes the tests. But I am still
concerned there is some unknown case that is impacted by us half-filling
out the tm_mon and tm_mday fields of the "struct tm" in the first half
of is_date.

-- >8 --
Subject: approxidate: allow ISO-like dates far in the future

When we are parsing approxidate strings and we find three
numbers separate by one of ":/-.", we guess that it may be a
date. We feed the numbers to match_multi_number, which
checks whether it makes sense as a date in various orderings
(e.g., dd/mm/yy or mm/dd/yy, etc).

One of the checks we do is to see whether it is a date more
than 10 days in the future. This was added in 38035cf (date
parsing: be friendlier to our European friends.,
2006-04-05), and lets us guess that if it is currently April
2014, then "10/03/2014" is probably March 10th, not October
3rd.

This has a downside, though; if you want to be overly
generous with your "--until" date specification, we may
wrongly parse "2014-12-01" as "2014-01-12" (because the
latter is an in-the-past date). If the year is a future year
(i.e., both are future dates), it gets even weirder. Due to
the vagaries of approxidate, months _after_ the current date
(no matter the year) get flipped, but ones before do not.

This patch drops the "in the future" check for dates of this
form, letting us treat them always as yyyy-mm-dd, even if
they are in the future. This does not affect the normal
dd/mm/yyyy versus mm/dd/yyyy lookup, because this code path
only kicks in when the first number is greater than 70
(i.e., it must be a year, and cannot be either a date or a
month).

The one possible casualty is that "yyyy-dd-mm" is less
likely to be chosen over "yyyy-mm-dd". That's probably OK,
though because:

  1. The difference happens only when the date is in the
     future. Already we prefer yyyy-mm-dd for dates in the
     past.

  2. It's unclear whether anybody even uses yyyy-dd-mm
     regularly. It does not appear in lists of common date
     formats in Wikipedia[1,2].

  3. Even if (2) is wrong, it is better to prefer ISO-like
     dates, as that is consistent with what we use elsewhere
     in git.

[1] http://en.wikipedia.org/wiki/Date_and_time_representation_by_country
[2] http://en.wikipedia.org/wiki/Calendar_date

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
 date.c          | 4 ++--
 t/t0006-date.sh | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/date.c b/date.c
index e1a4d49..3eba2df 100644
--- a/date.c
+++ b/date.c
@@ -441,10 +441,10 @@ static int match_multi_number(unsigned long num, char c, const char *date,
 
 		if (num > 70) {
 			/* yyyy-mm-dd? */
-			if (is_date(num, num2, num3, refuse_future, now, tm))
+			if (is_date(num, num2, num3, NULL, now, tm))
 				break;
 			/* yyyy-dd-mm? */
-			if (is_date(num, num3, num2, refuse_future, now, tm))
+			if (is_date(num, num3, num2, NULL, now, tm))
 				break;
 		}
 		/* Our eastern European friends say dd.mm.yy[yy]
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index e53cf6d..fac0986 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -82,4 +82,7 @@ check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
 check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
 check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
 
+check_approxidate '2008-12-01' '2008-12-01 19:20:00'
+check_approxidate '2009-12-01' '2009-12-01 19:20:00'
+
 test_done
-- 
2.1.2.596.g7379948

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]