From: Junio C Hamano Date: Thu, 23 Apr 2020 20:29:21 GMT Subject: Re: [PATCH v4 3/4] date.c: skip fractional second part of ISO-8601 Message-ID: In-Reply-To: <8b18d0ee5d6f08394e54e16ca7618c687791a509.1587644889.git.congdanhqx@gmail.com> Đoàn Trần Công Danh writes: > - if (num3 < 0) > + if (num3 < 0) { > num3 = 0; > + } else if (*end == '.' && isdigit(end[1]) && > + tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1 && > + set_time(num, num2, num3, tm) == 0) { > + /* %Y%m%d is known, ignore fractional in HHMMSS. */ > + strtol(end + 1, &end, 10); > + } > if (set_time(num, num2, num3, tm) == 0) Hmmm. While calling set_time() on the same 4-tuple might be idempotent when it succeeds, the call with a potential new side effect in the conditional part leaves a bad taste in my mouth. I am wondering if the follwoing would be a more readable alternative. date.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/date.c b/date.c index f5d5a91208..3eb9b6032c 100644 --- a/date.c +++ b/date.c @@ -572,8 +572,16 @@ static int match_multi_number(timestamp_t num, char c, const char *date, case ':': if (num3 < 0) num3 = 0; - if (set_time(num, num2, num3, tm) == 0) + if (set_time(num, num2, num3, tm) == 0) { + /* + * Is the "HH:MM:SS" we just parsed followed by + * "." (fractinal second)? Discard it + * only when we already have YY/MM/DD. + */ + if (*end == '.' && isdigit(end[1]) && date_known(tm)) + (void) strtol(end + 1, &end, 10); break; + } return 0; case '-':