Re: [PATCH 1/2] gitweb: rename parse_date() to format_date()

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@gmail.com>
---
 gitweb/gitweb.perl |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

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.

Similarly here.

Same here.

-- 
Jakub Narebski
Poland