Re: [PATCH (BUGFIX)] gitweb: Fix handling of fractional timezones in parse_date

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH (BUGFIX)] gitweb: Fix handling of fractional timezones in parse_date

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:52

Kevin Cernekee [off-list ref] writes:
It's just a matter of personal preference, but I would find this
regexp slightly easier to read:

+               ($tz =~ m/^([+\-])([0-9]{2})([0-9]{2})$/);
I'd say "^([-+])(\d\d)(\d\d)$" makes it the most clear.
quoted
+       $tz_sign = ($tz_sign eq '-' ? -1 : +1);
+       my $local = $epoch + $tz_sign*($tz_hour + ($tz_min/60.0))*3600;
If you wanted to avoid floats, you could do something like:

+       my $local = $epoch + $tz_sign * ($tz_hour * 3600 + $tz_min * 60);
That is not just float-avoidance, but is much more logical.

	(($h * 60) + $m) * 60

may be even more logical and more readable, though.

Care to re-roll the patch?

Re: [PATCH (BUGFIX)] gitweb: Fix handling of fractional timezones in parse_date

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:50:54

Junio C Hamano wrote:
Kevin Cernekee [off-list ref] writes:
quoted
It's just a matter of personal preference, but I would find this
regexp slightly easier to read:

+               ($tz =~ m/^([+\-])([0-9]{2})([0-9]{2})$/);
I'd say "^([-+])(\d\d)(\d\d)$" makes it the most clear.
But what does 'digit character' mean?  Is "\d" Unicode-aware, because
if it is it might match other digits than 0-9?

#perl says:

<rindolf> ShadeHawk: hi.
<rindolf> ShadeHawk: \d matches Unicode digits in unicode contexts I think.
<rindolf> ShadeHawk: like the ones used for Written Arabic.
<rindolf> ShadeHawk: let's see.
<rindolf> perlbot: eval: ["٣" =~ /\d/ ? "Match" : "Nomatch"]
<perlbot> rindolf: ["Match"]
<rindolf> Yay!
<rindolf> That's three in http://en.wikipedia.org/wiki/Hindu%E2%80%93Arabic_numeral_system

Anyway gitweb uses \d in various regexps, so it shouldn't be worse
than it is now.  

[...]
Care to re-roll the patch?
Will do. 

-- 
Jakub Narebski
ShadeHawk on #git and #perl
Poland

[PATCH (BUGFIX) v3] gitweb: Fix handling of fractional timezones in parse_date

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:50:54

Fractional timezones, like -0330 (NST used in Canada) or +0430
(Afghanistan, Iran DST), were not handled properly in parse_date; this
means values such as 'minute_local' and 'iso-tz' were not generated
correctly.

This was caused by two mistakes:

* sign of timezone was applied only to hour part of offset, and not
  as it should be also to minutes part (this affected only negative
  fractional timezones).

* 'int $h + $m/60' is 'int($h + $m/60)' and not 'int($h) + $m/60',
  so fractional part was discarded altogether ($h is hours, $m is
  minutes, which is always less than 60).


Note that positive fractional timezones +0430, +0530 and +1030 can be
found as authortime in git.git repository itself.

For example http://repo.or.cz/w/git.git/commit/88d50e7 had authortime
of "Fri, 8 Jan 2010 18:48:07 +0000 (23:48 +0530)", which is not marked
with 'atnight', when "git show 88d50e7" gives correct author date of
"Sat Jan 9 00:18:07 2010 +0530".

Signed-off-by: Jakub Narebski <redacted>
---
Junio C Hamano wrote:
Kevin Cernekee [off-list ref] writes:
quoted
It's just a matter of personal preference, but I would find this
regexp slightly easier to read:

+               ($tz =~ m/^([+\-])([0-9]{2})([0-9]{2})$/);
I'd say "^([-+])(\d\d)(\d\d)$" makes it the most clear.
quoted
quoted
+       $tz_sign = ($tz_sign eq '-' ? -1 : +1);
+       my $local = $epoch + $tz_sign*($tz_hour + ($tz_min/60.0))*3600;
If you wanted to avoid floats, you could do something like:

+       my $local = $epoch + $tz_sign * ($tz_hour * 3600 + $tz_min * 60);
That is not just float-avoidance, but is much more logical.

	(($h * 60) + $m) * 60

may be even more logical and more readable, though.

Care to re-roll the patch?
Re-rolled.

 gitweb/gitweb.perl |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0178633..ee69ea6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2921,8 +2921,10 @@ sub parse_date {
 	$date{'iso-8601'}  = sprintf "%04d-%02d-%02dT%02d:%02d:%02dZ",
 	                     1900+$year, 1+$mon, $mday, $hour ,$min, $sec;
 
-	$tz =~ m/^([+\-][0-9][0-9])([0-9][0-9])$/;
-	my $local = $epoch + ((int $1 + ($2/60)) * 3600);
+	my ($tz_sign, $tz_hour, $tz_min) =
+		($tz =~ m/^([-+])(\d\d)(\d\d)$/);
+	$tz_sign = ($tz_sign eq '-' ? -1 : +1);
+	my $local = $epoch + $tz_sign*((($tz_hour*60) + $tz_min)*60);
 	($sec, $min, $hour, $mday, $mon, $year, $wday, $yday) = gmtime($local);
 	$date{'hour_local'} = $hour;
 	$date{'minute_local'} = $min;
-- 
1.7.3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help