From: Danh Doan Date: Wed, 15 Apr 2020 02:15:21 GMT Subject: Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601 Message-ID: <20200415021521.GB19025@danh.dev> In-Reply-To: <20200414201606.GA1879688@coredump.intra.peff.net> On 2020-04-14 16:16:06-0400, Jeff King wrote: > On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote: > > > git-commit(1) says ISO-8601 is one of our supported date format. > > However, we only support RFC-3339 date format. > > > > We can either: > > - Update documentation from ISO-8601 to RFC-3339 > > - Add full support for ISO-8601 > > > > This series will try to add full support for ISO-8601. > > That seems more like a cover letter for the full series. The important > thing in this patch is more like: > > ISO-8601 allows timestamps to have a fractional number of seconds. We > represent time only in terms of whole seconds, so we never bothered > parsing fractional seconds. However, it's better for us to parse and > throw away the fractional part than to refuse to parse the timestamp > at all. Yeah, this sounds better. I intended write only this patch. I made the second patch when the comment about 8 digits for YYYYMMDD come to my eyes, hence this clumsy. > > diff --git a/date.c b/date.c > > index b0d9a8421d..2f37397beb 100644 > > --- a/date.c > > +++ b/date.c > > @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date, > > case ':': > > if (num3 < 0) > > num3 = 0; > > + else if (*end == '.' && isdigit(end[1])) > > + strtol(end + 1, &end, 10); > > I was a bit worried about this change hurting other cases where people > might use dots to separate numbers (but not mean a fraction). But the > two common ones I can think of should be OK: > > - 5.seconds.ago would never match because you look for a digit after > the dot This is covered. > - a date like 2019.10.12 wouldn't match, because we're only looking > after a ":", which heavily implies a time. Yeah, It's worth to add this test. -- Danh