Re: [PATCH] apply: Recognize epoch timestamps with : in the timezone

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 13, 2010, 18:59 UTC
Message-ID: <7v4ocpncli.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100929214107.GA4485@capella.cs.uchicago.edu>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 23 quoted lines

> Hi Anders,
>
> Anders Kaseorg wrote:
>> Some patches have a timezone formatted like '-08:00' instead of
>> '-0800' (e.g. http://lwn.net/Articles/131729/)
>
> Odd.  Any idea what tool generates these patches?
>
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
> [...]
>> @@ -765,7 +765,9 @@ static int has_epoch_timestamp(const char *nameline)
>>  	}
>>  
>>  	zoneoffset = strtol(timestamp + m[3].rm_so + 1, NULL, 10);
>> -	zoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);
>> +	if (m[4].rm_so == m[3].rm_so + 3)
>> +		zoneoffset /= 100;
>> +	zoneoffset = zoneoffset * 60 + strtol(timestamp + m[4].rm_so, NULL, 10);
>
> Might be clearer to write
>
> 	if (timestamp[m[3].rm_so + 3] != ':')

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])

so $3 is always 3 letters long (i.e. hour with sign), no? IOW, zoneoffset is never divided by 100 by the original patch.

What am I missing?