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