On Sat, 19 Mar 2011, Kevin Cernekee wrote:
> 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");
First, one would expect parse_date() to return either list of year,
month, etc., e.g. [2011, 3, 19, ...], or a hash with named
date fragments, e.g. {'year' => 2011, 'month' => 3, ...}.
Almost all other parse_* subroutines in gitweb (parse_commit, parse_tag,
parse_difftree_raw_line, parse_ls_tree_line, with the sole exception of
parse_from_to_diffinfo with perhaps a bit misleading name) return hash
of parsed and pre-formatted fragments.
So if parse_date(1300505805) returned
(
'hour' => 3,
'minute' => 36,
...
)
_without_ the 'rfc2822', 'mday-time' (unused), 'iso-8601', 'iso-tz',
it would be not misnamed parse_*.
Second, it is _date_ because it parses / processes dates in git internal
format. Perhaps it should be called *_raw_date, or *_git_date, or *_epoch,
or *_timestamp and not *_date.
> But instead, gitweb's parse_date works more like:
>
> &parse_date(1300505805) = {
Note: parse_date takes second argument, which is numeric timezone (defaults
to '-0000'), so one usually uses
parse_date(1300505805, '+0200')
> 'hour' => 3,
> 'minute' => 36,
> ...
> 'rfc2822' => 'Sat, 19 Mar 2011 03:36:45 +0000',
> ...
> }
What I would like to see is to split parsing date from generating formatted
dates into e.g. parse_date and formatted_date, and replace calls to parse_date
with calls to formatted_date (which uses parse_date internally).
> Rename the function [to format_date] to improve clarity.
> No change to functionality.
But name *format_date* is _not_ a good name. All other format_*
subroutines in gitweb return _formatted string_, not a hash which contains
many formatted strings.
It is hard to come up with really good name, but perhaps one from the ;ist
below would be good idea:
- formatted_date
- process_date, process_raw_date, process_git_date
- process_epoch, process_timestamp
- date_formats
- from_epoch, date_from_epoch
Any other ideas?
> Signed-off-by: Kevin Cernekee <cernekee@xxxxxxxxx>
> ---
> gitweb/gitweb.perl | 18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)
> @@ -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,
Hmm, we should probably fix it so it reads
- my %ad = parse_date($co{'author_epoch'});
+ my %ad = formatted_date($co{'author_epoch'}, $co{'author_tz'});
Especially when preparing for 'localtime' feature.
> @@ -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;
Similarly here.
> @@ -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,
Same here.
--
Jakub Narebski
Poland
--
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