GNU bug report logs - #14097
date: add parsing support for ISO 8601 basic format

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Mihai Capotă <mihai <at> mihaic.ro>
Cc: 14097 <at> debbugs.gnu.org, bug-gnulib <at> gnu.org
Subject: bug#14097: [PATCH] Add support for ISO 8601 basic format
Date: Wed, 24 Apr 2013 15:02:25 -0700

Thanks for taking this on.  Here is a brief review.
The most important thing is that the patch also needs
to update doc/parse-datetime.texi.  Also, some comments
about the code changes:

On 03/30/13 12:18, Mihai Capotă wrote:
> +      /* not ISO 8601 time, forcing mktime error */
> +      pc->hour = 90;

How does this force a mktime error?  mktime allows tm_hour == 90.

>  datetime:
>      iso_8601_datetime
> +  | iso_8601_basic_datetime
>    ;
>  
>  iso_8601_datetime:
>      iso_8601_date 'T' iso_8601_time
>    ;
>  
> +iso_8601_basic_datetime:
> +    number 'T' iso_8601_basic_time
> +      { pc->dates_seen--; } /* already incremented in digits_to_date_time */

This doesn't look right.  'number' accepts all sort of things that we
would rather not accept here.  Conversely, why require ":" in times to
correlate with "-" in dates?  Shouldn't we accept a "-"less date along
with a ":"ful time, and vice versa?  And that "dates_seen--" business
is a hack; can't we arrange things so that dates_seen is incremented
just once?

> +iso_8601_basic_time:
> +    tUNUMBER o_zone_offset
> +      {
> +        set_hhmmss_iso_8601_basic_time (pc, $1.value, 0);
> +        pc->meridian = MER24;
> +      }
> +  | tUDECIMAL_NUMBER o_zone_offset
> +      {
> +        /* FIXME avoid time_t to long int cast */

Why is the cast needed?  Also, can't the grammar be simplified
here, by using unsigned_seconds instead of using both
tUDECIMAL_NUMBER and tUNUMBER?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.