Jakub Narebski <jnareb@xxxxxxxxx> writes:
> Kevin, how about something like this instead? This preserves _intent_
> for why there is local time beside GMT time when 'localtime' is disabled
> better, I think.
>
> Junio and Kevin, I am not sure if authorship should remain with Kevin,
> or should it revert to me; the solution is quite different.
> About no-change to git_print_authorship: alternate solution would be
> to remove support for -localtime option, like in original patches.
I don't think it is worth anything to keep dead code that anybody
exercises to support -localtime option that nobody asks.
I thought we were getting closer (especially if you consider my suggestion
to the earlier round, but obviously I am biased), but this looks far worse
than your previous clean-up of Kevin's patch. What is the point of
duplicating the atnight logic her? Why not kill the useless helper
function "print-local-time", and instead enhance "format-local-time" so
that whatever this added code does is performed there when the caller asks?
Then the caller here would look more or less like:
print "<tr><td>$who</td>" .
"... author name, email, avatar ..." .
"<td></td><td>" .
format_timestamp(\%wd, gitweb_check_feature('localtime')) .
"</td></tr>\n";
and format_timestamp would be like
sub format_timestamp {
my %date = %$_[0];
my $use_localtime = $_[1];
my $localtime, $ret, $nite;
$nite = ($date{'hour_local'} < 6);
if ($use_localtime) {
$ret = $date{'rfc2822_local'};
if ($nite) {
$ret = sprintf("<span class='atnight'>%s</span>", $ret);
}
} else {
... what the current format_local_time does to set
... including the spanning part
$ret = "$date{'rfc2822'} ($localtime)";
}
return $ret;
}
Wouldn't it be much cleaner? You can then clean up the other call site of
print_local_time in git_print_authorship using the same helper function
(presumably you would always pass 0 to $use_localtime there), no?
> gitweb/gitweb.perl | 16 ++++++++++++----
> 1 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index cdc2a96..5bda0a8 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4003,15 +4003,23 @@ sub git_print_authorship_rows {
> my %wd = parse_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"})) . " " .
> + esc_html($co->{"${who}_name"})) . " " .
> format_search_author($co->{"${who}_email"}, $who,
> - esc_html("<" . $co->{"${who}_email"} . ">")) .
> + esc_html("<" . $co->{"${who}_email"} . ">")) .
> "</td><td rowspan=\"2\">" .
> git_get_avatar($co->{"${who}_email"}, -size => 'double') .
> "</td></tr>\n" .
> "<tr>" .
> - "<td></td><td> $wd{'rfc2822'}";
> - print_local_time(%wd) if !gitweb_check_feature('localtime');
> + "<td></td><td> ";
> + if (gitweb_check_feature('localtime')) {
> + if ($wd{'hour_local'} < 6) {
> + print "<span class=\"atnight\">$wd{'rfc2822'}</span>";
> + } else {
> + print $wd{'rfc2822'};
> + }
> + } else {
> + print $wd{'rfc2822'} . format_local_time(%wd);
> + }
> print "</td>" .
> "</tr>\n";
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html