Re: [PATCH v2 04/11] gitweb: optional custom name for refs in git_heads_body

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 13 Nov 2008, Giuseppe Bilotta wrote:

> We make a clear separation between the hash reference and the displayed
> name for refs displayed by git_heads_body. This can be used e.g. to
> group them and display only the distinct part of the name.

It is not clear for me from this commit message what this patch
is meant to do. Already git_heads_body (and also git_tags_body)
uses $ref{'name'} for display, and $ref{'fullname'} for linking
(to avoid possibility of tag/branch name conflict).

> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index ab29aec..a736f2a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4288,16 +4288,18 @@ sub git_heads_body {
>  		} else {
>  			print "<tr class=\"light\">\n";
>  		}
> +		my $hname = $ref{'hname'} || $ref{'fullname'} || $ref{'name'};

I don't remember setting $ref{'hname'} anywhere; if there is a patch
that sets this, it should really be squashed together with this commit.
Otherwise the commit is not standalone, as it should be.

> +		my $name = $ref{'name'};

I understand that this is simply "shortcut" name, to avoid using
$ref{'name'} everywhere else, and instead using $name?  This is a bit
independent, I mean in the sense that it makes sense to squash those
patches together in a kind of "by the way" way, i.e. simplify the code
if we are making changes in this area.

>  		$alternate ^= 1;
>  		print "<td><i>$ref{'age'}</i></td>\n" .
>  		      ($curr ? "<td class=\"current_head\">" : "<td>") .
> -		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'fullname'}),
> -		               -class => "list name"},esc_html($ref{'name'})) .
> +		      $cgi->a({-href => href(action=>"shortlog", hash=>$hname),
> +		               -class => "list name"},esc_html($name)) .
>  		      "</td>\n" .
>  		      "<td class=\"link\">" .
> -		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'fullname'})}, "shortlog") . " | " .
> -		      $cgi->a({-href => href(action=>"log", hash=>$ref{'fullname'})}, "log") . " | " .
> -		      $cgi->a({-href => href(action=>"tree", hash=>$ref{'fullname'}, hash_base=>$ref{'name'})}, "tree") .
> +		      $cgi->a({-href => href(action=>"shortlog", hash=>$hname)}, "shortlog") . " | " .
> +		      $cgi->a({-href => href(action=>"log", hash=>$hname)}, "log") . " | " .
> +		      $cgi->a({-href => href(action=>"tree", hash=>$hname, hash_base=>$hname)}, "tree") .
>  		      "</td>\n" .
>  		      "</tr>";
>  	}

So, in short I think this patch needs work, at least better commit
message (perhaps I don't understand something...)

-- 
Jakub Narebski
Poland