[PATCH v2 1/3] gitweb: rename parse_date() to format_date()

[PATCH v2 1/3] gitweb: rename parse_date() to format_date()

[Kevin Cernekee]

One might reasonably expect a function named parse_date() to be used
for something along these lines:

$unix_time_t = parse_date("2011-03-19");

But instead, gitweb's parse_date works more like:

&parse_date(1300505805, "-0800") = {
        'hour' => 3,
        'minute' => 36,
        ...
        'rfc2822' => 'Sat, 19 Mar 2011 03:36:45 +0000',
        ...
}

Rename the function to improve clarity.  No change to functionality.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---

v2: Add "-0800" to the commit message.  No code changes.

 gitweb/gitweb.perl |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b04ab8c..57ef08c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2893,7 +2893,7 @@ sub git_get_rev_name_tags {
 ## ----------------------------------------------------------------------
 ## parse to hash functions
 
-sub parse_date {
+sub format_date {
 	my $epoch = shift;
 	my $tz = shift || "-0000";
 
@@ -3953,7 +3953,7 @@ sub git_print_authorship {
 	my $tag = $opts{-tag} || 'div';
 	my $author = $co->{'author_name'};
 
-	my %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});
+	my %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});
 	print "<$tag class=\"author_date\">" .
 	      format_search_author($author, "author", esc_html($author)) .
 	      " [$ad{'rfc2822'}";
@@ -3973,7 +3973,7 @@ sub git_print_authorship_rows {
 	my @people = @_;
 	@people = ('author', 'committer') unless @people;
 	foreach my $who (@people) {
-		my %wd = parse_date($co->{"${who}_epoch"}, $co->{"${who}_tz"});
+		my %wd = format_date($co->{"${who}_epoch"}, $co->{"${who}_tz"});
 		print "<tr><td>$who</td><td>" .
 		      format_search_author($co->{"${who}_name"}, $who,
 			       esc_html($co->{"${who}_name"})) . " " .
@@ -4906,7 +4906,7 @@ sub git_log_body {
 		next if !%co;
 		my $commit = $co{'id'};
 		my $ref = format_ref_marker($refs, $commit);
-		my %ad = parse_date($co{'author_epoch'});
+		my %ad = format_date($co{'author_epoch'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
 		               esc_html($co{'title'}) . $ref,
@@ -5369,7 +5369,7 @@ sub git_project_index {
 sub git_summary {
 	my $descr = git_get_project_description($project) || "none";
 	my %co = parse_commit("HEAD");
-	my %cd = %co ? parse_date($co{'committer_epoch'}, $co{'committer_tz'}) : ();
+	my %cd = %co ? format_date($co{'committer_epoch'}, $co{'committer_tz'}) : ();
 	my $head = $co{'id'};
 	my $remote_heads = gitweb_check_feature('remote_heads');
 
@@ -5674,7 +5674,7 @@ sub git_blame_common {
 			my $short_rev = substr($full_rev, 0, 8);
 			my $author = $meta->{'author'};
 			my %date =
-				parse_date($meta->{'author-time'}, $meta->{'author-tz'});
+				format_date($meta->{'author-time'}, $meta->{'author-tz'});
 			my $date = $date{'iso-tz'};
 			if ($group_size) {
 				$current_color = ($current_color + 1) % $num_colors;
@@ -6702,7 +6702,7 @@ sub git_commitdiff {
 			-charset => 'utf-8',
 			-expires => $expires,
 			-content_disposition => 'inline; filename="' . "$filename" . '"');
-		my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
+		my %ad = format_date($co{'author_epoch'}, $co{'author_tz'});
 		print "From: " . to_utf8($co{'author'}) . "\n";
 		print "Date: $ad{'rfc2822'} ($ad{'tz_local'})\n";
 		print "Subject: " . to_utf8($co{'title'}) . "\n";
@@ -7064,7 +7064,7 @@ sub git_feed {
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
 		my $latest_epoch = $latest_commit{'committer_epoch'};
-		%latest_date   = parse_date($latest_epoch);
+		%latest_date   = format_date($latest_epoch);
 		my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
 		if (defined $if_modified) {
 			my $since;
@@ -7195,7 +7195,7 @@ XML
 		if (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {
 			last;
 		}
-		my %cd = parse_date($co{'author_epoch'});
+		my %cd = format_date($co{'author_epoch'});
 
 		# get list of changed files
 		open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-- 
1.7.4.1

[PATCH v5 2/3] gitweb: introduce localtime feature

[Kevin Cernekee]

With this feature enabled, all timestamps are shown in the local
timezone instead of GMT.  The timezone is taken from the appropriate
timezone string stored in the commit object.

This improves usability if the majority of a project's contributors are
based in a single office, all within the same timezone.  It also makes
the interface more friendly to non-developers who may need to track
updates, such as program managers and supervisors.

This change does not affect relative timestamps (e.g. "5 hours ago"),
nor does it affect 'patch' and 'patches' views which already use
localtime because they are generated by "git format-patch".

Affected views include:
* 'summary' view, "last change" field (commit time from latest change)
* 'log' view, author time
* 'commit' and 'commitdiff' views, author/committer time
* 'tag' view, tagger time

In the case of 'commit', 'commitdiff' and 'tag' views, gitweb used to
print both GMT time and time in timezone of author/tagger/committer:

   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)

With localtime enabled, the times will be swapped:

   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)

When the local time is between 00:00 and 05:59, inclusive, the entire
date string will be printed in red ("atnight" style) on the commit and
commitdiff views.  This is a change from the current behavior, in which
only the parenthesized time/tz is colored red.  This simplifies the
code, and makes the special coloring easier to notice.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

v5: Change "$use_localtime" to "$commit_view" for clarity.  Apply
"atnight" style to the entire timestamp string, not just the
" (hh:mm -zzzz)" portion.  Update commit message to reflect the
latter change.

 gitweb/gitweb.perl |   84 +++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 57ef08c..aa038bd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -504,6 +504,19 @@ our %feature = (
 		'sub' => sub { feature_bool('remote_heads', @_) },
 		'override' => 0,
 		'default' => [0]},
+
+	# Use the author/commit localtime rather than GMT for all timestamps.
+	# Disabled by default.
+
+	# To enable system wide have in $GITWEB_CONFIG
+	# $feature{'localtime'}{'default'} = [1];
+	# To have project specific config enable override in $GITWEB_CONFIG
+	# $feature{'localtime'}{'override'} = 1;
+	# and in project config gitweb.localtime = 0|1;
+	'localtime' => {
+		'sub' => sub { feature_bool('localtime', @_) },
+		'override' => 0,
+		'default' => [0]},
 );
 
 sub gitweb_get_feature {
@@ -2919,6 +2932,10 @@ sub format_date {
 	$date{'hour_local'} = $hour;
 	$date{'minute_local'} = $min;
 	$date{'tz_local'} = $tz;
+	$date{'rfc2822_local'} = sprintf "%s, %d %s %4d %02d:%02d:%02d $tz",
+	                         $days[$wday], $mday, $months[$mon],
+	                         1900+$year, $hour ,$min, $sec;
+
 	$date{'iso-tz'} = sprintf("%04d-%02d-%02d %02d:%02d:%02d %s",
 	                          1900+$year, $mon+1, $mday,
 	                          $hour, $min, $sec, $tz);
@@ -3928,22 +3945,43 @@ sub git_print_section {
 	print $cgi->end_div;
 }
 
-sub print_local_time {
-	print format_local_time(@_);
-}
-
-sub format_local_time {
-	my $localtime = '';
-	my %date = @_;
-	if ($date{'hour_local'} < 6) {
-		$localtime .= sprintf(" (<span class=\"atnight\">%02d:%02d</span> %s)",
-			$date{'hour_local'}, $date{'minute_local'}, $date{'tz_local'});
+# Returns a timestamp string, which may contain HTML.
+# If $commit_view is 0, the string looks like:
+#   Fri, 18 Mar 2011 01:28:57 +0000                [localtime disabled]
+#   Thu, 17 Mar 2011 18:28:57 -0700                [localtime enabled]
+# If $commit_view is 1, the string looks like:
+#   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)  [localtime disabled]
+#   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)  [localtime enabled]
+# If $commit_view is 1, the entire string will use the "atnight" class
+# (red text) if the local time is between 00:00 and 05:59 inclusive.
+# This helps to flag commits made in the wee hours of the morning.
+sub timestamp_html {
+	my ($date, $commit_view) = @_;
+	my $timestamp;
+	my $alt_time;
+
+	if (gitweb_check_feature('localtime')) {
+		$timestamp = $date->{'rfc2822_local'};
+		$alt_time = sprintf(" (%02d:%02d %s)",
+		                    $date->{'hour'},
+		                    $date->{'minute'},
+		                    "+0000");
 	} else {
-		$localtime .= sprintf(" (%02d:%02d %s)",
-			$date{'hour_local'}, $date{'minute_local'}, $date{'tz_local'});
+		$timestamp = $date->{'rfc2822'};
+		$alt_time = sprintf(" (%02d:%02d %s)",
+		                    $date->{'hour_local'},
+		                    $date->{'minute_local'},
+		                    $date->{'tz_local'});
 	}
-
-	return $localtime;
+	if ($commit_view) {
+		$timestamp .= $alt_time;
+		if ($date->{'hour_local'} < 6) {
+			$timestamp = "<span class=\"atnight\">" .
+			             $timestamp .
+			             "</span>";
+		}
+	}
+	return $timestamp;
 }
 
 # Outputs the author name and date in long form
@@ -3956,10 +3994,9 @@ sub git_print_authorship {
 	my %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});
 	print "<$tag class=\"author_date\">" .
 	      format_search_author($author, "author", esc_html($author)) .
-	      " [$ad{'rfc2822'}";
-	print_local_time(%ad) if ($opts{-localtime});
-	print "]" . git_get_avatar($co->{'author_email'}, -pad_before => 1)
-		  . "</$tag>\n";
+	      " [" . timestamp_html(\%ad, 0) . "] ".
+	      git_get_avatar($co->{'author_email'}, -pad_before => 1) .
+	      "</$tag>\n";
 }
 
 # Outputs table rows containing the full author or committer information,
@@ -3983,9 +4020,9 @@ sub git_print_authorship_rows {
 		      git_get_avatar($co->{"${who}_email"}, -size => 'double') .
 		      "</td></tr>\n" .
 		      "<tr>" .
-		      "<td></td><td> $wd{'rfc2822'}";
-		print_local_time(%wd);
-		print "</td>" .
+		      "<td></td><td> " .
+		      timestamp_html(\%wd, 1) .
+		      "</td>" .
 		      "</tr>\n";
 	}
 }
@@ -5395,8 +5432,9 @@ sub git_summary {
 	print "<table class=\"projects_list\">\n" .
 	      "<tr id=\"metadata_desc\"><td>description</td><td>" . esc_html($descr) . "</td></tr>\n" .
 	      "<tr id=\"metadata_owner\"><td>owner</td><td>" . esc_html($owner) . "</td></tr>\n";
-	if (defined $cd{'rfc2822'}) {
-		print "<tr id=\"metadata_lchange\"><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n";
+	if (keys %cd) {
+		print "<tr id=\"metadata_lchange\"><td>last change</td><td>" .
+		      timestamp_html(\%cd, 0) . "</td></tr>\n";
 	}
 
 	# use per project git URL list in $projectroot/$project/cloneurl
-- 
1.7.4.1

[PATCH/RFC 3/3] gitweb: change highlighting of "atnight" commits

[Kevin Cernekee]

The current behavior is to show the timestamp for "atnight" commits
(commits for which the local time is between 00:00 and 05:59,
inclusive) in red, on the commit and commitdiff views.  On the
log view, the timestamps are shown normally.

Swap this behavior so that the commit and commitdiff views do
not color these timestamps red, but the log view does.  This allows
late-night commits to be easily identified (and possibly subjected to
extra scrutiny) when reading through the change log.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---

Personally, I am not too concerned with the "atnight" behavior because
only a minuscule percentage of our commits happen late at night.  But
since we're touching this code, we might as well tweak anything in there
that needs tweaking.

 gitweb/gitweb.perl |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index aa038bd..2d4349f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3946,17 +3946,17 @@ sub git_print_section {
 }
 
 # Returns a timestamp string, which may contain HTML.
-# If $commit_view is 0, the string looks like:
+# If -commit_view is not specified, the string looks like:
 #   Fri, 18 Mar 2011 01:28:57 +0000                [localtime disabled]
 #   Thu, 17 Mar 2011 18:28:57 -0700                [localtime enabled]
-# If $commit_view is 1, the string looks like:
+# If -commit_view is specified, the string looks like:
 #   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)  [localtime disabled]
 #   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)  [localtime enabled]
-# If $commit_view is 1, the entire string will use the "atnight" class
+# If -atnight is specified, the entire string will use the "atnight" class
 # (red text) if the local time is between 00:00 and 05:59 inclusive.
 # This helps to flag commits made in the wee hours of the morning.
 sub timestamp_html {
-	my ($date, $commit_view) = @_;
+	my ($date, %opts) = @_;
 	my $timestamp;
 	my $alt_time;
 
@@ -3973,13 +3973,13 @@ sub timestamp_html {
 		                    $date->{'minute_local'},
 		                    $date->{'tz_local'});
 	}
-	if ($commit_view) {
+	if ($opts{-commit_view}) {
 		$timestamp .= $alt_time;
-		if ($date->{'hour_local'} < 6) {
-			$timestamp = "<span class=\"atnight\">" .
-			             $timestamp .
-			             "</span>";
-		}
+	}
+	if ($opts{-atnight} && $date->{'hour_local'} < 6) {
+		$timestamp = "<span class=\"atnight\">" .
+			     $timestamp .
+			     "</span>";
 	}
 	return $timestamp;
 }
@@ -3994,7 +3994,7 @@ sub git_print_authorship {
 	my %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});
 	print "<$tag class=\"author_date\">" .
 	      format_search_author($author, "author", esc_html($author)) .
-	      " [" . timestamp_html(\%ad, 0) . "] ".
+	      " [" . timestamp_html(\%ad, -atnight => 1) . "] ".
 	      git_get_avatar($co->{'author_email'}, -pad_before => 1) .
 	      "</$tag>\n";
 }
@@ -4021,7 +4021,7 @@ sub git_print_authorship_rows {
 		      "</td></tr>\n" .
 		      "<tr>" .
 		      "<td></td><td> " .
-		      timestamp_html(\%wd, 1) .
+		      timestamp_html(\%wd, -commit_view => 1) .
 		      "</td>" .
 		      "</tr>\n";
 	}
@@ -5434,7 +5434,7 @@ sub git_summary {
 	      "<tr id=\"metadata_owner\"><td>owner</td><td>" . esc_html($owner) . "</td></tr>\n";
 	if (keys %cd) {
 		print "<tr id=\"metadata_lchange\"><td>last change</td><td>" .
-		      timestamp_html(\%cd, 0) . "</td></tr>\n";
+		      timestamp_html(\%cd) . "</td></tr>\n";
 	}
 
 	# use per project git URL list in $projectroot/$project/cloneurl
-- 
1.7.4.1

[PATCH -1/3] gitweb: Always call parse_date with timezone parameter

[Jakub Narebski]

Timezone is required to correctly set local time, which would be needed
for future 'localtime' feature.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b04ab8c..0f89ed2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4906,7 +4906,7 @@ sub git_log_body {
 		next if !%co;
 		my $commit = $co{'id'};
 		my $ref = format_ref_marker($refs, $commit);
-		my %ad = parse_date($co{'author_epoch'});
+		my %ad = parse_date($co{'author_epoch'}, $co{'athor_tz'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
 		               esc_html($co{'title'}) . $ref,
@@ -7064,7 +7064,7 @@ sub git_feed {
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
 		my $latest_epoch = $latest_commit{'committer_epoch'};
-		%latest_date   = parse_date($latest_epoch);
+		%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});
 		my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
 		if (defined $if_modified) {
 			my $since;
@@ -7195,7 +7195,7 @@ XML
 		if (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {
 			last;
 		}
-		my %cd = parse_date($co{'author_epoch'});
+		my %cd = parse_date($co{'author_epoch'}, $co{'author_tz'});
 
 		# get list of changed files
 		open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-- 
1.7.3

[PATCH -1/3 (amend)] gitweb: Always call parse_date with timezone parameter

[Jakub Narebski]

Timezone is required to correctly set local time, which would be needed
for future 'localtime' feature.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I am very sorry, a typo snuck in in previous version

BTW. Kevin, when there is series of patches which are dependent on each
other and create together a logical whole, it is recommended to provide
cover letter for such "true series".

 gitweb/gitweb.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b04ab8c..16abd4c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4906,7 +4906,7 @@ sub git_log_body {
 		next if !%co;
 		my $commit = $co{'id'};
 		my $ref = format_ref_marker($refs, $commit);
-		my %ad = parse_date($co{'author_epoch'});
+		my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
 		               esc_html($co{'title'}) . $ref,
@@ -7064,7 +7064,7 @@ sub git_feed {
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
 		my $latest_epoch = $latest_commit{'committer_epoch'};
-		%latest_date   = parse_date($latest_epoch);
+		%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});
 		my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
 		if (defined $if_modified) {
 			my $since;
@@ -7195,7 +7195,7 @@ XML
 		if (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {
 			last;
 		}
-		my %cd = parse_date($co{'author_epoch'});
+		my %cd = parse_date($co{'author_epoch'}, $co{'author_tz'});
 
 		# get list of changed files
 		open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-- 
1.7.3

Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter

[Kevin Cernekee]

On Sat, Mar 19, 2011 at 3:18 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> @@ -4906,7 +4906,7 @@ sub git_log_body {
>                next if !%co;
>                my $commit = $co{'id'};
>                my $ref = format_ref_marker($refs, $commit);
> -               my %ad = parse_date($co{'author_epoch'});
> +               my %ad = parse_date($co{'author_epoch'}, $co{'athor_tz'});

Should be 'author_tz'

Looking at the master branch, I don't see %ad actually getting used
anywhere?  Maybe it is safe to delete the line entirely, since
git_print_authorship() calls parse_date() itself.

> @@ -7064,7 +7064,7 @@ sub git_feed {
>        if (defined($commitlist[0])) {
>                %latest_commit = %{$commitlist[0]};
>                my $latest_epoch = $latest_commit{'committer_epoch'};
> -               %latest_date   = parse_date($latest_epoch);
> +               %latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});

Should be 'committer_tz'

I would agree that it isn't such a good thing for
$latest_date{'rfc2822_local'} to be set to GMT in this case.  Although
the feeds don't need local times for anything, since RSS/Atom readers
seem to do their own timezone translations.

It probably makes sense to add this argument so that nobody gets bit
later when they try to use the rfc2822_local field.

Am I correct in interpreting "PATCH -1/3" as: "apply this before
Kevin's set of 3 patches?"

Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter

[Jakub Narebski]

Kevin Cernekee wrote:

> Am I correct in interpreting "PATCH -1/3" as: "apply this before
> Kevin's set of 3 patches?"

Well, it must be applied before change of name from parse_date...
or it would have to be changed.

-- 
Jakub Narebski
Poland

Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter

[Junio C Hamano]

Kevin Cernekee <cernekee@gmail.com> writes:

> On Sat, Mar 19, 2011 at 3:18 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> ...
> Should be 'author_tz'
>
> Looking at the master branch, I don't see %ad actually getting used
> anywhere?  Maybe it is safe to delete the line entirely, since
> git_print_authorship() calls parse_date() itself.

Thanks for being careful, I agree "while at it, remove an unnecessary
call" would be better.

>> @@ -7064,7 +7064,7 @@ sub git_feed {
> ...
> Should be 'committer_tz'
>
> I would agree that it isn't such a good thing for
> $latest_date{'rfc2822_local'} to be set to GMT in this case.

I would say it isn't good in _any_ case.

If the localtime conversion logic _were_ very expensive, and if the
function that fills 'rfc2822_local' _were_ written to allow callers that
do not need local time by passing undef to its tz parameter to avoid the
computation cost, it may make sense to allow 'rfc2822_local' to be undef,
but neither is true.  This callsite was simply buggy.

> Am I correct in interpreting "PATCH -1/3" as: "apply this before
> Kevin's set of 3 patches?"

I had the same reaction; judging from the contents, I now realize that is
what was meant by "minus one", but originally I read that hyphen as "I
don't know how many iterations we had so instead of writing v$N I am just
striking it out" ;-)

[PATCH v2 4/3] gitweb: Always call format_date with timezone parameter

[Kevin Cernekee]

From: Jakub Narebski <jnareb@gmail.com>

Make the timezone parameter mandatory.  This ensures that the *_local
fields are always populated with accurate information.

Also, delete an unnecessary call to format_date().

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---

v2: Fix typos.  Remove unnecessary call.  Remove default "-0000" tz value.
Rebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a
merge conflict).

 gitweb/gitweb.perl |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2d4349f..485ebd0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2907,8 +2907,7 @@ sub git_get_rev_name_tags {
 ## parse to hash functions
 
 sub format_date {
-	my $epoch = shift;
-	my $tz = shift || "-0000";
+	my ($epoch, $tz) = @_;
 
 	my %date;
 	my @months = ("Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec");
@@ -4943,7 +4942,6 @@ sub git_log_body {
 		next if !%co;
 		my $commit = $co{'id'};
 		my $ref = format_ref_marker($refs, $commit);
-		my %ad = format_date($co{'author_epoch'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
 		               esc_html($co{'title'}) . $ref,
@@ -7102,7 +7100,8 @@ sub git_feed {
 	if (defined($commitlist[0])) {
 		%latest_commit = %{$commitlist[0]};
 		my $latest_epoch = $latest_commit{'committer_epoch'};
-		%latest_date   = format_date($latest_epoch);
+		%latest_date   = format_date($latest_epoch,
+		                             $latest_commit{'committer_tz'});
 		my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
 		if (defined $if_modified) {
 			my $since;
@@ -7233,7 +7232,7 @@ XML
 		if (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {
 			last;
 		}
-		my %cd = format_date($co{'author_epoch'});
+		my %cd = format_date($co{'author_epoch'}, $co{'author_tz'});
 
 		# get list of changed files
 		open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-- 
1.7.4.1

Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter

[Jakub Narebski]

On Sun, 20 Mar 2011, Kevin Cernekee wrote:
> From: Jakub Narebski <jnareb@gmail.com>
> 
> Make the timezone parameter mandatory.  This ensures that the *_local
> fields are always populated with accurate information.

That is better commit message that what I came with.
 
> Also, delete an unnecessary call to format_date().
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
> ---
> 
> v2: Fix typos.  Remove unnecessary call.

Thanks.

> Remove default "-0000" tz value.

Hmmm... I wonder if default value for timezone might have been intended
to handle case of malformed date, without timezone.

This fragment

        my $epoch = shift;
        my $tz = shift || "-0000";

is from very beginnings of gitweb... and "v042" is a bit uninformative
as a commit message.  991910a (v042, 2005-08-07).

> Rebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a
> merge conflict).

You are right, that is better solution.

-- 
Jakub Narebski
Poland

Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter

[Jakub Narebski]

Jakub Narebski wrote:
> On Sun, 20 Mar 2011, Kevin Cernekee wrote:

> > Rebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a
> > merge conflict).
> 
> You are right, that is better solution.

Well, better for you, as it makes things easier for you: no need to
resolve conflict, or to rerun update script:

  $ git checkout HEAD^ -- gitweb/gitweb.perl
  $ sed -e 's/\bparse_date\b/\bformat_date\b/' \
    <gitweb/gitweb.perl >gitweb/gitweb.perl+  &&
    mv -f gitweb/gitweb.perl+ gitweb/gitweb.perl

But it takes bugfix hostage to applying new feature series.
-- 
Jakub Narebski
Poland

Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter

[Kevin Cernekee]

On Sun, Mar 20, 2011 at 4:07 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> But it takes bugfix hostage to applying new feature series.

I can reorder and resubmit if needed...

But, since the bug isn't actually showing any symptoms in the existing
code, and since the fix only depends on patch 1/3 ("rename parse_date"
cleanup) rather than 2/3 + 3/3 (localtime + atnight features), I'm
hoping this won't be a major point of contention.