From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 13 Oct 2010 23:37:15 GMT
Subject: Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone
Message-ID: <7vvd55fyvo.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20101013225012.GB9509@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Neither the patch nor your suggestion makes much sense to me.  With the
>>> patch, the regexp is now
>>>
>>>     ^(1969-12-31|1970-01-01) <time>(\.0+)? ([-+][0-2][0-9]):?([0-5][0-9])
> [...]
>> Well, I was missing that without ':' strtol() goes through to parse $3$4
>> as a single integer
>
> So maybe something like the following would make this easier to follow.

Yeah, this feels much more natural.

> diff --git a/builtin/apply.c b/builtin/apply.c
> index 0fa9a8d..000d3e5 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -733,8 +733,8 @@ static int has_epoch_timestamp(const char *nameline)
>  		" "
>  		"[0-2][0-9]:[0-5][0-9]:00(\\.0+)?"
>  		" "
> -		"([-+][0-2][0-9]):?([0-5][0-9])\n";
> +		"([-+][0-2][0-9]:?[0-5][0-9])\n";
> -	const char *timestamp = NULL, *cp;
> +	const char *timestamp = NULL, *cp, *colon;
>  	static regex_t *stamp;
>  	regmatch_t m[10];
>  	int zoneoffset;
> @@ -764,10 +764,11 @@ static int has_epoch_timestamp(const char *nameline)
>  		return 0;
>  	}
>  
> -	zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
> +	zoneoffset = strtol(timestamp + m[3].rm_so + 1, (char **) &colon, 10);
> -	if (m[4].rm_so == m[3].rm_so + 3)
> -		zoneoffset /= 100;
> -	zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
> +	if (*colon == ':')
> +		zoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);
> +	else
> +		zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
>  	if (timestamp[m[3].rm_so] == '-')
>  		zoneoffset = -zoneoffset;
>