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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakub Narebski <jnareb@xxxxxxxxx> writes:

> 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',
);

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]