Re: [PATCH v2 04/11] gitweb: optional custom name for refs in git_heads_body
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Nov 14, 2008, 23:32 UTC
- Message-ID
- <200811150032.14558.jnareb@gmail.com>
- In-Reply-To
- <1226616555-24503-5-git-send-email-giuseppe.bilotta@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).
Show 15 quoted lines
>
> 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.
Show 18 quoted lines
> $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