Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 15, 2020, 15:03 UTC
Message-ID: <xmqq5ze094gg.fsf@gitster.c.googlers.com>
In-Reply-To: <20200414202401.GC1879688@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> On Tue, Apr 14, 2020 at 04:31:55PM +0700, Đoàn Trần Công Danh wrote:
>
>> @@ -666,6 +666,24 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
>>  		n++;
>>  	} while (isdigit(date[n]));
>>  
>> +	/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */
>> +	if (n == 8) {
>> +		tm->tm_year = num / 10000 - 1900;
>> +		tm->tm_mon = (num % 10000) / 100 - 1;
>> +		tm->tm_mday = num % 100;
>> +		return n;
>> +	}
>
> I worry a little this may conflict with other approxidate heuristics.
>
> The only one I can think of is an actual unix timestamp, though, and we
> already require that to have at least 9 digits (plus anybody wanting to
> use one robustly really should be using @12345678).

I am OK with 1/2, but I'd worry a LOT about this one, if we didn't require 9 digits. 60/100 * 60/100 * 24/100 ~= 8.6% so limiting the hour/min/sec to sensible values is not a useful protection against ambiguity. We'd essentially be declaring that a raw UNIX timestamp without @ prefix is now hit-and-miss if we take this change, were there not the "must be at least 9 digits" requirement.

Somebody was thinking when he wrote the very beginning part of the match_digit() function ;-)

> We could probably tighten the heuristics a bit by insisting that the
> month and day be sensible. Or even year (see the 1900 to 2100 magic for
> the 4-digit year guess).

I do agree that we'd want 0 <= hr <= 24, 0 <= min <= 59, and 0 <= sec <= 60 (or should this be 61?), but it is for correctness of the new code. It wouldn't have any value in disambiguation from other formats, I would think. So, from that point of view, I would buy something like 1969 as a lower bound, but I am not sure if we necessarily want an upper bound for the year. What mistake are we protecting us against?

Thanks.

Show 13 quoted lines

>> +	/* 6 digits, compact style of ISO-8601's time: HHMMSS */
>> +	if (n == 6) {
>> +		tm->tm_hour = num / 10000;
>> +		tm->tm_min = (num % 10000) / 100;
>> +		tm->tm_sec = num % 100;
>> +		if (*end == '.' && isdigit(end[1]))
>> +			strtoul(end + 1, &end, 10);
>> +		return end - date;
>> +	}
>
> And likewise here that the hour, minute, and second be reasonable.
>
> -Peff