Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation

From: Junio C Hamano <junkio@cox.net>
Date: Oct 7, 2006, 08:36 UTC
Message-ID: <7viriwwmid.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200610061231.06017.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 8 quoted lines

> Rename $githelp_url and $githelp_label to $logo_url and $logo_label to
> be more obvious what they refer to; while at it add commented out
> previous contents (git documentation at kernel.org). Add comment about
> logo size.
>
> Use $cgi->a(...) to generate Git logo link; it automatically escapes
> attribute values when it is needed.  Escape href attribute using
> esc_url instead of (incorrect!) esc_html.

In principle, I am not sure renaming the variables is a good idea in general. Renaming them means you are burning people who have custom help link defined in their gitweb_config.perl file, so unless the new variable names are ten times better than the current one I do not think it is worth doing.

These variables, however, have never been in any official release nor -rc yet, and have only been there for exactly two weeks, so if we are going to rename them, we'd better do so now. And the variables are about the same thing described by $logo variable, so the new names probably make more sense.

Having said that, I am wondering if it is worth doing something like this:

sub img_button {
	my $it = $_[0];
	my @attr = ();
        for my $attr (qw(src width height alt class id)) {
		next unless exists $it->{$attr};
                push @attr, "$attr=\"" . esc_attr($it->{$attr}) . '"';
	}
	my $img = '<img ' . join(' ', @attr) . ' />';
	print $cgi->a({ -href => esc_url($it->{'url'}),
        		-title => esc_attr($it->{'title'}),
                      }, $img);
}

our %logo = (
	# logo image button
	src => '++GITWEB_LOGO++',
        width => 72,
        height => 27,
	alt => 'git logo',

	# where that link leads to
        url => 'http:/git.or.cz/',
	title => 'git homepage',
);