bug-gnulib
[Top][All Lists]
Advanced

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

Re: [PATCH]: Parse ISO 8601 extended date and time of day format


From: Jim Meyering
Subject: Re: [PATCH]: Parse ISO 8601 extended date and time of day format
Date: Thu, 18 Aug 2011 23:25:34 +0200

J.T. Conklin wrote:
> Quite a long time ago, I posted a proof of concept change to
> parse-datetime.y to enable parsing of ISO 8601 "extended date and time
> of day expressions" using a 'T' separator character.
>
> While waiting for my copyright assignment paperwork to clear, I found
> and fixed some corner cases related to the 'T' characters use as both
> a military zone token and the date/time separator; added some test
> cases; and tweaked the documentation.
>
> Not content to leave well enough alone, I started adding support for
> "basic date and time of day expressions", got stuck in grammar
> conflict hell, and ran out of spare time.
>
> This change rolls things back to support extended format, the 'T'
> corner case fixes, test cases, and doc changes.
>
> Many thanks to Jim Meyering who's been waiting very patiently and
> gently nudging me to complete this patch.
>
> Subject: [PATCH] Parse ISO 8601 extended date and time of day format
>
> * doc/parse-datetime.texi (General date syntax): replace use of
> deprecated --iso-8601 option with --rfc-3339 in example of date
> command output formats that can be parsed.
>
> * tests/test-parse-datetime.c (tm_diff): New function, taken from
> lib/parse-datetime.y.
> (gmt_offset): New function.
> (main): Add additional test cases to validate ISO8601 extended
> date and time of day format parsing.
>
> * lib/parse-datetime.y: Parse ISO 8601 extended date and time
> of day representation using the 'T' separator character.

Thanks for what looks a patch with impeccable code.
It passed the basic gnulib-tool test:

  ./gnulib-tool --create-testdir --with-tests --test parse-datetime

The above directory also passed coverity (though I had to
comment out the two verify uses to make coverity succeed in
analyzing parse-datetime.c).

I would change two doc-related things:

  - adjust the one-line summary to include the module name and put all
    "* ..." lines in the same paragraph. (I've already adjusted that, as below)

  - I noticed that the new format <iso 8601 date> "T" <iso 8601 date>
    is not documented.  There's already a sentence or two on the
    @sc{iso} 8601 date format.  Dare I ask? ... Would you care
    to add a few words about the new one?  If so, and if you send
    me a simple patch, I'll merge it into the existing commit.



commit bf2f6889923b05dea312e58962997afe28ee27a3
Author: J.T. Conklin <address@hidden>
Date:   Wed Aug 17 16:40:49 2011 -0700

    parse-datetime: accept ISO 8601 date and time rep with "T" separator

    * doc/parse-datetime.texi (General date syntax): replace use of
    deprecated --iso-8601 option with --rfc-3339 in example of date
    command output formats that can be parsed.
    * tests/test-parse-datetime.c (tm_diff): New function, taken from
    lib/parse-datetime.y.
    (gmt_offset): New function.
    (main): Add additional test cases to validate ISO8601 extended
    date and time of day format parsing.
    * lib/parse-datetime.y: Parse ISO 8601 extended date and time
    of day representation using the 'T' separator character.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]