Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601
- From
Danh Doan <congdanhqx@gmail.com>
- Date
- Apr 15, 2020, 02:15 UTC
- 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 <peff@peff.net> wrote:
Show 19 quoted lines
> 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.
Show 17 quoted lines
> > 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