Re: [PATCH 2/2] Accept the timezone specifiers [+-]hh:mm and [+-]hh in addition to [+-]hhmm

[Junio C Hamano <gitster@pobox.com>]

Marcus Comstedt <marcus@mc.pp.se> writes:

> ISO 8601 specifies three syntaxes for timezones other than "Z".
> git already supports the +-hhmm syntax.  This patch adds support
> for the other two: +-hh:mm and +-hh.
>
> Signed-off-by: Marcus Comstedt <marcus@mc.pp.se>
> ---
> :100644 100644 6bae49c... f83e46e... M	date.c
>  date.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/date.c b/date.c
> index 6bae49c..f83e46e 100644
> --- a/date.c
> +++ b/date.c
> @@ -555,6 +555,18 @@ static int match_tz(const char *date, int *offp)
>  	int min, hour;
>  	int n = end - date - 1;
>  
> +	/* Check for HH:MM format, allowed by ISO 8601 */
> +	if (n == 2 && date[3] == ':') {
> +		char *end2;
> +		min = strtoul(date+4, &end2, 10);
> +		/* If we have two digits after the colon too, assume HH:MM */
> +		if (end2 == date+6) {
> +			offset = offset*100 + min;
> +			end = end2;
> +			n = end - date - 1;
> +		}
> +	}
> +
>  	min = offset % 100;
>  	hour = offset / 100;
>  
> @@ -570,6 +582,17 @@ static int match_tz(const char *date, int *offp)
>  
>  		*offp = offset;
>  	}
> +	/*
> +	 * Also accept just the hour, allowed by ISO 8601
> +	 */
> +	else if (n == 2 && hour == 0 && min < 24) {
> +		offset = min*60;
> +		if (*date == '-')
> +			offset = -offset;
> +
> +		*offp = offset;
> +	}
> +

I don't recall seeing in ISO 8601 that +hh or -hh without minute
resolution was allowed, but I don't have my copy of ISO 8601 with me (they
are packed and are still in transit with my household goods) so I'll take
your word for it for now [*1*].

But the placement of this second hunk is somewhat curious.  Why doesn't the
updated function look like this?

        int offset = strtoul(date + 1, &end, 10);
        int min, hour;
        int n = end - date - 1;

        if (n == 2 && offset <= 14) {
                /* +HH:MM (ISO 8601) or +HH (ISO 8601 abbreviated) */
                hour = offset;
                if (n == 2 && date[3] == ':') {
                        min = strtoul(date + 4, &end, 10);
                        if (end != date + 6)
                                return 0; /* Bad CRAP */
                } else {
                        min = 0;
                }
        } else if (n < 3) {
                return 0; /* we want at least 3 digits */
        } else {
                min = offset % 100;
                hour = offset / 100;
        }

        if (60 <= min)
                return 0; /* invalid minute */

        offset = hour * 60 + min;
        if (*date == '-')
                offset = -offset;
        *offp = offset;
        return end - date;


[Footnote]

*1* Appendix A of RFC3339 seems to agree with you.