{"thread":{"id":"26796","subject":"[PATCH v2 1/3] gitweb: rename parse_date() to format_date()","startedAt":"2011-03-19T20:48:25Z","lastAt":"2011-03-20T20:47:52Z","messageCount":12,"participants":["Kevin Cernekee","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"163776","messageId":"4f21902cf5f72b30a96465cf911d13aa@localhost","threadId":"26796","inReplyTo":null,"subject":"[PATCH v2 1/3] gitweb: rename parse_date() to format_date()","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-19T20:48:25Z","receivedAt":"2011-03-19T20:48:25Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"One might reasonably expect a function named parse_date() to be used\nfor something along these lines:\n\n$unix_time_t = parse_date(\"2011-03-19\");\n\nBut instead, gitweb's parse_date works more like:\n\n&parse_date(1300505805, \"-0800\") = {\n        'hour' => 3,\n        'minute' => 36,\n        ...\n        'rfc2822' => 'Sat, 19 Mar 2011 03:36:45 +0000',\n        ...\n}\n\nRename the function to improve clarity.  No change to functionality.\n\nSigned-off-by: Kevin Cernekee <cernekee@gmail.com>\n---\n\nv2: Add \"-0800\" to the commit message.  No code changes.\n\n gitweb/gitweb.perl |   18 +++++++++---------\n 1 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b04ab8c..57ef08c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2893,7 +2893,7 @@ sub git_get_rev_name_tags {\n ## ----------------------------------------------------------------------\n ## parse to hash functions\n \n-sub parse_date {\n+sub format_date {\n \tmy $epoch = shift;\n \tmy $tz = shift || \"-0000\";\n \n@@ -3953,7 +3953,7 @@ sub git_print_authorship {\n \tmy $tag = $opts{-tag} || 'div';\n \tmy $author = $co->{'author_name'};\n \n-\tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n+\tmy %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});\n \tprint \"<$tag class=\\\"author_date\\\">\" .\n \t      format_search_author($author, \"author\", esc_html($author)) .\n \t      \" [$ad{'rfc2822'}\";\n@@ -3973,7 +3973,7 @@ sub git_print_authorship_rows {\n \tmy @people = @_;\n \t@people = ('author', 'committer') unless @people;\n \tforeach my $who (@people) {\n-\t\tmy %wd = parse_date($co->{\"${who}_epoch\"}, $co->{\"${who}_tz\"});\n+\t\tmy %wd = format_date($co->{\"${who}_epoch\"}, $co->{\"${who}_tz\"});\n \t\tprint \"<tr><td>$who</td><td>\" .\n \t\t      format_search_author($co->{\"${who}_name\"}, $who,\n \t\t\t       esc_html($co->{\"${who}_name\"})) . \" \" .\n@@ -4906,7 +4906,7 @@ sub git_log_body {\n \t\tnext if !%co;\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %ad = parse_date($co{'author_epoch'});\n+\t\tmy %ad = format_date($co{'author_epoch'});\n \t\tgit_print_header_div('commit',\n \t\t               \"<span class=\\\"age\\\">$co{'age_string'}</span>\" .\n \t\t               esc_html($co{'title'}) . $ref,\n@@ -5369,7 +5369,7 @@ sub git_project_index {\n sub git_summary {\n \tmy $descr = git_get_project_description($project) || \"none\";\n \tmy %co = parse_commit(\"HEAD\");\n-\tmy %cd = %co ? parse_date($co{'committer_epoch'}, $co{'committer_tz'}) : ();\n+\tmy %cd = %co ? format_date($co{'committer_epoch'}, $co{'committer_tz'}) : ();\n \tmy $head = $co{'id'};\n \tmy $remote_heads = gitweb_check_feature('remote_heads');\n \n@@ -5674,7 +5674,7 @@ sub git_blame_common {\n \t\t\tmy $short_rev = substr($full_rev, 0, 8);\n \t\t\tmy $author = $meta->{'author'};\n \t\t\tmy %date =\n-\t\t\t\tparse_date($meta->{'author-time'}, $meta->{'author-tz'});\n+\t\t\t\tformat_date($meta->{'author-time'}, $meta->{'author-tz'});\n \t\t\tmy $date = $date{'iso-tz'};\n \t\t\tif ($group_size) {\n \t\t\t\t$current_color = ($current_color + 1) % $num_colors;\n@@ -6702,7 +6702,7 @@ sub git_commitdiff {\n \t\t\t-charset => 'utf-8',\n \t\t\t-expires => $expires,\n \t\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"');\n-\t\tmy %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});\n+\t\tmy %ad = format_date($co{'author_epoch'}, $co{'author_tz'});\n \t\tprint \"From: \" . to_utf8($co{'author'}) . \"\\n\";\n \t\tprint \"Date: $ad{'rfc2822'} ($ad{'tz_local'})\\n\";\n \t\tprint \"Subject: \" . to_utf8($co{'title'}) . \"\\n\";\n@@ -7064,7 +7064,7 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = parse_date($latest_epoch);\n+\t\t%latest_date   = format_date($latest_epoch);\n \t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n \t\tif (defined $if_modified) {\n \t\t\tmy $since;\n@@ -7195,7 +7195,7 @@ XML\n \t\tif (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {\n \t\t\tlast;\n \t\t}\n-\t\tmy %cd = parse_date($co{'author_epoch'});\n+\t\tmy %cd = format_date($co{'author_epoch'});\n \n \t\t# get list of changed files\n \t\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-- \n1.7.4.1\n"},{"id":"163778","messageId":"d0b137d3ad11b997ed0964354bd9017a@localhost","threadId":"26796","inReplyTo":"4f21902cf5f72b30a96465cf911d13aa@localhost","subject":"[PATCH v5 2/3] gitweb: introduce localtime feature","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-19T20:48:26Z","receivedAt":"2011-03-19T20:48:26Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"With this feature enabled, all timestamps are shown in the local\ntimezone instead of GMT.  The timezone is taken from the appropriate\ntimezone string stored in the commit object.\n\nThis improves usability if the majority of a project's contributors are\nbased in a single office, all within the same timezone.  It also makes\nthe interface more friendly to non-developers who may need to track\nupdates, such as program managers and supervisors.\n\nThis change does not affect relative timestamps (e.g. \"5 hours ago\"),\nnor does it affect 'patch' and 'patches' views which already use\nlocaltime because they are generated by \"git format-patch\".\n\nAffected views include:\n* 'summary' view, \"last change\" field (commit time from latest change)\n* 'log' view, author time\n* 'commit' and 'commitdiff' views, author/committer time\n* 'tag' view, tagger time\n\nIn the case of 'commit', 'commitdiff' and 'tag' views, gitweb used to\nprint both GMT time and time in timezone of author/tagger/committer:\n\n   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)\n\nWith localtime enabled, the times will be swapped:\n\n   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)\n\nWhen the local time is between 00:00 and 05:59, inclusive, the entire\ndate string will be printed in red (\"atnight\" style) on the commit and\ncommitdiff views.  This is a change from the current behavior, in which\nonly the parenthesized time/tz is colored red.  This simplifies the\ncode, and makes the special coloring easier to notice.\n\nSigned-off-by: Kevin Cernekee <cernekee@gmail.com>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n\nv5: Change \"$use_localtime\" to \"$commit_view\" for clarity.  Apply\n\"atnight\" style to the entire timestamp string, not just the\n\" (hh:mm -zzzz)\" portion.  Update commit message to reflect the\nlatter change.\n\n gitweb/gitweb.perl |   84 +++++++++++++++++++++++++++++++++++++--------------\n 1 files changed, 61 insertions(+), 23 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 57ef08c..aa038bd 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -504,6 +504,19 @@ our %feature = (\n \t\t'sub' => sub { feature_bool('remote_heads', @_) },\n \t\t'override' => 0,\n \t\t'default' => [0]},\n+\n+\t# Use the author/commit localtime rather than GMT for all timestamps.\n+\t# Disabled by default.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'localtime'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'localtime'}{'override'} = 1;\n+\t# and in project config gitweb.localtime = 0|1;\n+\t'localtime' => {\n+\t\t'sub' => sub { feature_bool('localtime', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -2919,6 +2932,10 @@ sub format_date {\n \t$date{'hour_local'} = $hour;\n \t$date{'minute_local'} = $min;\n \t$date{'tz_local'} = $tz;\n+\t$date{'rfc2822_local'} = sprintf \"%s, %d %s %4d %02d:%02d:%02d $tz\",\n+\t                         $days[$wday], $mday, $months[$mon],\n+\t                         1900+$year, $hour ,$min, $sec;\n+\n \t$date{'iso-tz'} = sprintf(\"%04d-%02d-%02d %02d:%02d:%02d %s\",\n \t                          1900+$year, $mon+1, $mday,\n \t                          $hour, $min, $sec, $tz);\n@@ -3928,22 +3945,43 @@ sub git_print_section {\n \tprint $cgi->end_div;\n }\n \n-sub print_local_time {\n-\tprint format_local_time(@_);\n-}\n-\n-sub format_local_time {\n-\tmy $localtime = '';\n-\tmy %date = @_;\n-\tif ($date{'hour_local'} < 6) {\n-\t\t$localtime .= sprintf(\" (<span class=\\\"atnight\\\">%02d:%02d</span> %s)\",\n-\t\t\t$date{'hour_local'}, $date{'minute_local'}, $date{'tz_local'});\n+# Returns a timestamp string, which may contain HTML.\n+# If $commit_view is 0, the string looks like:\n+#   Fri, 18 Mar 2011 01:28:57 +0000                [localtime disabled]\n+#   Thu, 17 Mar 2011 18:28:57 -0700                [localtime enabled]\n+# If $commit_view is 1, the string looks like:\n+#   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)  [localtime disabled]\n+#   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)  [localtime enabled]\n+# If $commit_view is 1, the entire string will use the \"atnight\" class\n+# (red text) if the local time is between 00:00 and 05:59 inclusive.\n+# This helps to flag commits made in the wee hours of the morning.\n+sub timestamp_html {\n+\tmy ($date, $commit_view) = @_;\n+\tmy $timestamp;\n+\tmy $alt_time;\n+\n+\tif (gitweb_check_feature('localtime')) {\n+\t\t$timestamp = $date->{'rfc2822_local'};\n+\t\t$alt_time = sprintf(\" (%02d:%02d %s)\",\n+\t\t                    $date->{'hour'},\n+\t\t                    $date->{'minute'},\n+\t\t                    \"+0000\");\n \t} else {\n-\t\t$localtime .= sprintf(\" (%02d:%02d %s)\",\n-\t\t\t$date{'hour_local'}, $date{'minute_local'}, $date{'tz_local'});\n+\t\t$timestamp = $date->{'rfc2822'};\n+\t\t$alt_time = sprintf(\" (%02d:%02d %s)\",\n+\t\t                    $date->{'hour_local'},\n+\t\t                    $date->{'minute_local'},\n+\t\t                    $date->{'tz_local'});\n \t}\n-\n-\treturn $localtime;\n+\tif ($commit_view) {\n+\t\t$timestamp .= $alt_time;\n+\t\tif ($date->{'hour_local'} < 6) {\n+\t\t\t$timestamp = \"<span class=\\\"atnight\\\">\" .\n+\t\t\t             $timestamp .\n+\t\t\t             \"</span>\";\n+\t\t}\n+\t}\n+\treturn $timestamp;\n }\n \n # Outputs the author name and date in long form\n@@ -3956,10 +3994,9 @@ sub git_print_authorship {\n \tmy %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});\n \tprint \"<$tag class=\\\"author_date\\\">\" .\n \t      format_search_author($author, \"author\", esc_html($author)) .\n-\t      \" [$ad{'rfc2822'}\";\n-\tprint_local_time(%ad) if ($opts{-localtime});\n-\tprint \"]\" . git_get_avatar($co->{'author_email'}, -pad_before => 1)\n-\t\t  . \"</$tag>\\n\";\n+\t      \" [\" . timestamp_html(\\%ad, 0) . \"] \".\n+\t      git_get_avatar($co->{'author_email'}, -pad_before => 1) .\n+\t      \"</$tag>\\n\";\n }\n \n # Outputs table rows containing the full author or committer information,\n@@ -3983,9 +4020,9 @@ sub git_print_authorship_rows {\n \t\t      git_get_avatar($co->{\"${who}_email\"}, -size => 'double') .\n \t\t      \"</td></tr>\\n\" .\n \t\t      \"<tr>\" .\n-\t\t      \"<td></td><td> $wd{'rfc2822'}\";\n-\t\tprint_local_time(%wd);\n-\t\tprint \"</td>\" .\n+\t\t      \"<td></td><td> \" .\n+\t\t      timestamp_html(\\%wd, 1) .\n+\t\t      \"</td>\" .\n \t\t      \"</tr>\\n\";\n \t}\n }\n@@ -5395,8 +5432,9 @@ sub git_summary {\n \tprint \"<table class=\\\"projects_list\\\">\\n\" .\n \t      \"<tr id=\\\"metadata_desc\\\"><td>description</td><td>\" . esc_html($descr) . \"</td></tr>\\n\" .\n \t      \"<tr id=\\\"metadata_owner\\\"><td>owner</td><td>\" . esc_html($owner) . \"</td></tr>\\n\";\n-\tif (defined $cd{'rfc2822'}) {\n-\t\tprint \"<tr id=\\\"metadata_lchange\\\"><td>last change</td><td>$cd{'rfc2822'}</td></tr>\\n\";\n+\tif (keys %cd) {\n+\t\tprint \"<tr id=\\\"metadata_lchange\\\"><td>last change</td><td>\" .\n+\t\t      timestamp_html(\\%cd, 0) . \"</td></tr>\\n\";\n \t}\n \n \t# use per project git URL list in $projectroot/$project/cloneurl\n-- \n1.7.4.1\n"},{"id":"163777","messageId":"e420df635f1146388df59a27bdc6e68b@localhost","threadId":"26796","inReplyTo":"4f21902cf5f72b30a96465cf911d13aa@localhost","subject":"[PATCH/RFC 3/3] gitweb: change highlighting of \"atnight\" commits","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-19T20:48:27Z","receivedAt":"2011-03-19T20:48:27Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"The current behavior is to show the timestamp for \"atnight\" commits\n(commits for which the local time is between 00:00 and 05:59,\ninclusive) in red, on the commit and commitdiff views.  On the\nlog view, the timestamps are shown normally.\n\nSwap this behavior so that the commit and commitdiff views do\nnot color these timestamps red, but the log view does.  This allows\nlate-night commits to be easily identified (and possibly subjected to\nextra scrutiny) when reading through the change log.\n\nSigned-off-by: Kevin Cernekee <cernekee@gmail.com>\n---\n\nPersonally, I am not too concerned with the \"atnight\" behavior because\nonly a minuscule percentage of our commits happen late at night.  But\nsince we're touching this code, we might as well tweak anything in there\nthat needs tweaking.\n\n gitweb/gitweb.perl |   26 +++++++++++++-------------\n 1 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex aa038bd..2d4349f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3946,17 +3946,17 @@ sub git_print_section {\n }\n \n # Returns a timestamp string, which may contain HTML.\n-# If $commit_view is 0, the string looks like:\n+# If -commit_view is not specified, the string looks like:\n #   Fri, 18 Mar 2011 01:28:57 +0000                [localtime disabled]\n #   Thu, 17 Mar 2011 18:28:57 -0700                [localtime enabled]\n-# If $commit_view is 1, the string looks like:\n+# If -commit_view is specified, the string looks like:\n #   Fri, 18 Mar 2011 01:28:57 +0000 (18:28 -0700)  [localtime disabled]\n #   Thu, 17 Mar 2011 18:28:57 -0700 (01:28 +0000)  [localtime enabled]\n-# If $commit_view is 1, the entire string will use the \"atnight\" class\n+# If -atnight is specified, the entire string will use the \"atnight\" class\n # (red text) if the local time is between 00:00 and 05:59 inclusive.\n # This helps to flag commits made in the wee hours of the morning.\n sub timestamp_html {\n-\tmy ($date, $commit_view) = @_;\n+\tmy ($date, %opts) = @_;\n \tmy $timestamp;\n \tmy $alt_time;\n \n@@ -3973,13 +3973,13 @@ sub timestamp_html {\n \t\t                    $date->{'minute_local'},\n \t\t                    $date->{'tz_local'});\n \t}\n-\tif ($commit_view) {\n+\tif ($opts{-commit_view}) {\n \t\t$timestamp .= $alt_time;\n-\t\tif ($date->{'hour_local'} < 6) {\n-\t\t\t$timestamp = \"<span class=\\\"atnight\\\">\" .\n-\t\t\t             $timestamp .\n-\t\t\t             \"</span>\";\n-\t\t}\n+\t}\n+\tif ($opts{-atnight} && $date->{'hour_local'} < 6) {\n+\t\t$timestamp = \"<span class=\\\"atnight\\\">\" .\n+\t\t\t     $timestamp .\n+\t\t\t     \"</span>\";\n \t}\n \treturn $timestamp;\n }\n@@ -3994,7 +3994,7 @@ sub git_print_authorship {\n \tmy %ad = format_date($co->{'author_epoch'}, $co->{'author_tz'});\n \tprint \"<$tag class=\\\"author_date\\\">\" .\n \t      format_search_author($author, \"author\", esc_html($author)) .\n-\t      \" [\" . timestamp_html(\\%ad, 0) . \"] \".\n+\t      \" [\" . timestamp_html(\\%ad, -atnight => 1) . \"] \".\n \t      git_get_avatar($co->{'author_email'}, -pad_before => 1) .\n \t      \"</$tag>\\n\";\n }\n@@ -4021,7 +4021,7 @@ sub git_print_authorship_rows {\n \t\t      \"</td></tr>\\n\" .\n \t\t      \"<tr>\" .\n \t\t      \"<td></td><td> \" .\n-\t\t      timestamp_html(\\%wd, 1) .\n+\t\t      timestamp_html(\\%wd, -commit_view => 1) .\n \t\t      \"</td>\" .\n \t\t      \"</tr>\\n\";\n \t}\n@@ -5434,7 +5434,7 @@ sub git_summary {\n \t      \"<tr id=\\\"metadata_owner\\\"><td>owner</td><td>\" . esc_html($owner) . \"</td></tr>\\n\";\n \tif (keys %cd) {\n \t\tprint \"<tr id=\\\"metadata_lchange\\\"><td>last change</td><td>\" .\n-\t\t      timestamp_html(\\%cd, 0) . \"</td></tr>\\n\";\n+\t\t      timestamp_html(\\%cd) . \"</td></tr>\\n\";\n \t}\n \n \t# use per project git URL list in $projectroot/$project/cloneurl\n-- \n1.7.4.1\n"},{"id":"163786","messageId":"201103192318.45925.jnareb@gmail.com","threadId":"26796","inReplyTo":"4f21902cf5f72b30a96465cf911d13aa@localhost","subject":"[PATCH -1/3] gitweb: Always call parse_date with timezone parameter","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-19T22:18:44Z","receivedAt":"2011-03-19T22:18:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Timezone is required to correctly set local time, which would be needed\nfor future 'localtime' feature.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b04ab8c..0f89ed2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4906,7 +4906,7 @@ sub git_log_body {\n \t\tnext if !%co;\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %ad = parse_date($co{'author_epoch'});\n+\t\tmy %ad = parse_date($co{'author_epoch'}, $co{'athor_tz'});\n \t\tgit_print_header_div('commit',\n \t\t               \"<span class=\\\"age\\\">$co{'age_string'}</span>\" .\n \t\t               esc_html($co{'title'}) . $ref,\n@@ -7064,7 +7064,7 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = parse_date($latest_epoch);\n+\t\t%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n \t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n \t\tif (defined $if_modified) {\n \t\t\tmy $since;\n@@ -7195,7 +7195,7 @@ XML\n \t\tif (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {\n \t\t\tlast;\n \t\t}\n-\t\tmy %cd = parse_date($co{'author_epoch'});\n+\t\tmy %cd = parse_date($co{'author_epoch'}, $co{'author_tz'});\n \n \t\t# get list of changed files\n \t\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-- \n1.7.3\n"},{"id":"163789","messageId":"201103192353.56841.jnareb@gmail.com","threadId":"26796","inReplyTo":"201103192318.45925.jnareb@gmail.com","subject":"[PATCH -1/3 (amend)] gitweb: Always call parse_date with timezone parameter","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-19T22:53:55Z","receivedAt":"2011-03-19T22:53:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Timezone is required to correctly set local time, which would be needed\nfor future 'localtime' feature.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI am very sorry, a typo snuck in in previous version\n\nBTW. Kevin, when there is series of patches which are dependent on each\nother and create together a logical whole, it is recommended to provide\ncover letter for such \"true series\".\n\n gitweb/gitweb.perl |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b04ab8c..16abd4c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4906,7 +4906,7 @@ sub git_log_body {\n \t\tnext if !%co;\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %ad = parse_date($co{'author_epoch'});\n+\t\tmy %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});\n \t\tgit_print_header_div('commit',\n \t\t               \"<span class=\\\"age\\\">$co{'age_string'}</span>\" .\n \t\t               esc_html($co{'title'}) . $ref,\n@@ -7064,7 +7064,7 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = parse_date($latest_epoch);\n+\t\t%latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n \t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n \t\tif (defined $if_modified) {\n \t\t\tmy $since;\n@@ -7195,7 +7195,7 @@ XML\n \t\tif (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {\n \t\t\tlast;\n \t\t}\n-\t\tmy %cd = parse_date($co{'author_epoch'});\n+\t\tmy %cd = parse_date($co{'author_epoch'}, $co{'author_tz'});\n \n \t\t# get list of changed files\n \t\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-- \n1.7.3\n"},{"id":"163790","messageId":"AANLkTimV7vvD0PTMejydiyW_CeUH0cuQ-2+PnRqjzob5@mail.gmail.com","threadId":"26796","inReplyTo":"201103192318.45925.jnareb@gmail.com","subject":"Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-19T22:56:04Z","receivedAt":"2011-03-19T22:56:04Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"On Sat, Mar 19, 2011 at 3:18 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> @@ -4906,7 +4906,7 @@ sub git_log_body {\n>                next if !%co;\n>                my $commit = $co{'id'};\n>                my $ref = format_ref_marker($refs, $commit);\n> -               my %ad = parse_date($co{'author_epoch'});\n> +               my %ad = parse_date($co{'author_epoch'}, $co{'athor_tz'});\n\nShould be 'author_tz'\n\nLooking at the master branch, I don't see %ad actually getting used\nanywhere?  Maybe it is safe to delete the line entirely, since\ngit_print_authorship() calls parse_date() itself.\n\n> @@ -7064,7 +7064,7 @@ sub git_feed {\n>        if (defined($commitlist[0])) {\n>                %latest_commit = %{$commitlist[0]};\n>                my $latest_epoch = $latest_commit{'committer_epoch'};\n> -               %latest_date   = parse_date($latest_epoch);\n> +               %latest_date   = parse_date($latest_epoch, $latest_commit{'comitter_tz'});\n\nShould be 'committer_tz'\n\nI would agree that it isn't such a good thing for\n$latest_date{'rfc2822_local'} to be set to GMT in this case.  Although\nthe feeds don't need local times for anything, since RSS/Atom readers\nseem to do their own timezone translations.\n\nIt probably makes sense to add this argument so that nobody gets bit\nlater when they try to use the rfc2822_local field.\n\nAm I correct in interpreting \"PATCH -1/3\" as: \"apply this before\nKevin's set of 3 patches?\"\n"},{"id":"163792","messageId":"201103200125.59427.jnareb@gmail.com","threadId":"26796","inReplyTo":"AANLkTimV7vvD0PTMejydiyW_CeUH0cuQ-2+PnRqjzob5@mail.gmail.com","subject":"Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-20T00:25:57Z","receivedAt":"2011-03-20T00:25:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Kevin Cernekee wrote:\n\n> Am I correct in interpreting \"PATCH -1/3\" as: \"apply this before\n> Kevin's set of 3 patches?\"\n\nWell, it must be applied before change of name from parse_date...\nor it would have to be changed.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"163793","messageId":"7v39miws8p.fsf@alter.siamese.dyndns.org","threadId":"26796","inReplyTo":"AANLkTimV7vvD0PTMejydiyW_CeUH0cuQ-2+PnRqjzob5@mail.gmail.com","subject":"Re: [PATCH -1/3] gitweb: Always call parse_date with timezone parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-20T00:27:50Z","receivedAt":"2011-03-20T00:27:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Cernekee <cernekee@gmail.com> writes:\n\n> On Sat, Mar 19, 2011 at 3:18 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> ...\n> Should be 'author_tz'\n>\n> Looking at the master branch, I don't see %ad actually getting used\n> anywhere?  Maybe it is safe to delete the line entirely, since\n> git_print_authorship() calls parse_date() itself.\n\nThanks for being careful, I agree \"while at it, remove an unnecessary\ncall\" would be better.\n\n>> @@ -7064,7 +7064,7 @@ sub git_feed {\n> ...\n> Should be 'committer_tz'\n>\n> I would agree that it isn't such a good thing for\n> $latest_date{'rfc2822_local'} to be set to GMT in this case.\n\nI would say it isn't good in _any_ case.\n\nIf the localtime conversion logic _were_ very expensive, and if the\nfunction that fills 'rfc2822_local' _were_ written to allow callers that\ndo not need local time by passing undef to its tz parameter to avoid the\ncomputation cost, it may make sense to allow 'rfc2822_local' to be undef,\nbut neither is true.  This callsite was simply buggy.\n\n> Am I correct in interpreting \"PATCH -1/3\" as: \"apply this before\n> Kevin's set of 3 patches?\"\n\nI had the same reaction; judging from the contents, I now realize that is\nwhat was meant by \"minus one\", but originally I read that hyphen as \"I\ndon't know how many iterations we had so instead of writing v$N I am just\nstriking it out\" ;-)\n"},{"id":"163798","messageId":"b599dae39131b90d0970a1ef63e6599b@localhost","threadId":"26796","inReplyTo":"4f21902cf5f72b30a96465cf911d13aa@localhost","subject":"[PATCH v2 4/3] gitweb: Always call format_date with timezone parameter","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-20T02:11:16Z","receivedAt":"2011-03-20T02:11:16Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"From: Jakub Narebski <jnareb@gmail.com>\n\nMake the timezone parameter mandatory.  This ensures that the *_local\nfields are always populated with accurate information.\n\nAlso, delete an unnecessary call to format_date().\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Kevin Cernekee <cernekee@gmail.com>\n---\n\nv2: Fix typos.  Remove unnecessary call.  Remove default \"-0000\" tz value.\nRebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a\nmerge conflict).\n\n gitweb/gitweb.perl |    9 ++++-----\n 1 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2d4349f..485ebd0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2907,8 +2907,7 @@ sub git_get_rev_name_tags {\n ## parse to hash functions\n \n sub format_date {\n-\tmy $epoch = shift;\n-\tmy $tz = shift || \"-0000\";\n+\tmy ($epoch, $tz) = @_;\n \n \tmy %date;\n \tmy @months = (\"Jan\", \"Feb\", \"Mar\", \"Apr\", \"May\", \"Jun\", \"Jul\", \"Aug\", \"Sep\", \"Oct\", \"Nov\", \"Dec\");\n@@ -4943,7 +4942,6 @@ sub git_log_body {\n \t\tnext if !%co;\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %ad = format_date($co{'author_epoch'});\n \t\tgit_print_header_div('commit',\n \t\t               \"<span class=\\\"age\\\">$co{'age_string'}</span>\" .\n \t\t               esc_html($co{'title'}) . $ref,\n@@ -7102,7 +7100,8 @@ sub git_feed {\n \tif (defined($commitlist[0])) {\n \t\t%latest_commit = %{$commitlist[0]};\n \t\tmy $latest_epoch = $latest_commit{'committer_epoch'};\n-\t\t%latest_date   = format_date($latest_epoch);\n+\t\t%latest_date   = format_date($latest_epoch,\n+\t\t                             $latest_commit{'committer_tz'});\n \t\tmy $if_modified = $cgi->http('IF_MODIFIED_SINCE');\n \t\tif (defined $if_modified) {\n \t\t\tmy $since;\n@@ -7233,7 +7232,7 @@ XML\n \t\tif (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {\n \t\t\tlast;\n \t\t}\n-\t\tmy %cd = format_date($co{'author_epoch'});\n+\t\tmy %cd = format_date($co{'author_epoch'}, $co{'author_tz'});\n \n \t\t# get list of changed files\n \t\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-- \n1.7.4.1\n"},{"id":"163812","messageId":"201103201137.18619.jnareb@gmail.com","threadId":"26796","inReplyTo":"b599dae39131b90d0970a1ef63e6599b@localhost","subject":"Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-20T10:37:17Z","receivedAt":"2011-03-20T10:37:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 20 Mar 2011, Kevin Cernekee wrote:\n> From: Jakub Narebski <jnareb@gmail.com>\n> \n> Make the timezone parameter mandatory.  This ensures that the *_local\n> fields are always populated with accurate information.\n\nThat is better commit message that what I came with.\n \n> Also, delete an unnecessary call to format_date().\n> \n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>\n> ---\n> \n> v2: Fix typos.  Remove unnecessary call.\n\nThanks.\n\n> Remove default \"-0000\" tz value.\n\nHmmm... I wonder if default value for timezone might have been intended\nto handle case of malformed date, without timezone.\n\nThis fragment\n\n        my $epoch = shift;\n        my $tz = shift || \"-0000\";\n\nis from very beginnings of gitweb... and \"v042\" is a bit uninformative\nas a commit message.  991910a (v042, 2005-08-07).\n\n> Rebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a\n> merge conflict).\n\nYou are right, that is better solution.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"163814","messageId":"201103201207.56639.jnareb@gmail.com","threadId":"26796","inReplyTo":"201103201137.18619.jnareb@gmail.com","subject":"Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-03-20T11:07:55Z","receivedAt":"2011-03-20T11:07:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> On Sun, 20 Mar 2011, Kevin Cernekee wrote:\n\n> > Rebase on top of my patch 3/3 (as applying -1/3 then 1/3 would create a\n> > merge conflict).\n> \n> You are right, that is better solution.\n\nWell, better for you, as it makes things easier for you: no need to\nresolve conflict, or to rerun update script:\n\n  $ git checkout HEAD^ -- gitweb/gitweb.perl\n  $ sed -e 's/\\bparse_date\\b/\\bformat_date\\b/' \\\n    <gitweb/gitweb.perl >gitweb/gitweb.perl+  &&\n    mv -f gitweb/gitweb.perl+ gitweb/gitweb.perl\n\nBut it takes bugfix hostage to applying new feature series.\n-- \nJakub Narebski\nPoland\n"},{"id":"163837","messageId":"AANLkTi=R+03BQZxh+8pcvxKRjO00_8Eo8FJ31buWZ6rb@mail.gmail.com","threadId":"26796","inReplyTo":"201103201207.56639.jnareb@gmail.com","subject":"Re: [PATCH v2 4/3] gitweb: Always call format_date with timezone parameter","fromName":"Kevin Cernekee","fromEmail":"cernekee@gmail.com","sentAt":"2011-03-20T20:47:52Z","receivedAt":"2011-03-20T20:47:52Z","isPatch":true,"sender":{"key":"cernekee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1631864?v=4"},"body":"On Sun, Mar 20, 2011 at 4:07 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> But it takes bugfix hostage to applying new feature series.\n\nI can reorder and resubmit if needed...\n\nBut, since the bug isn't actually showing any symptoms in the existing\ncode, and since the fix only depends on patch 1/3 (\"rename parse_date\"\ncleanup) rather than 2/3 + 3/3 (localtime + atnight features), I'm\nhoping this won't be a major point of contention.\n"}]}