git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
@ 2006-09-19 21:27 Petr Baudis
  2006-09-23 12:57 ` Petr Baudis
  0 siblings, 1 reply; 25+ messages in thread
From: Petr Baudis @ 2006-09-19 21:27 UTC (permalink / raw)
  To: git

It provides more useful information for causual Git users than the Git docs
(especially about where to get Git and such).

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c77270c..7ecd7df 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1312,7 +1312,7 @@ EOF
 	print "</head>\n" .
 	      "<body>\n" .
 	      "<div class=\"page_header\">\n" .
-	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
+	      "<a href=\"http://git.or.cz/\" title=\"Git homepage\">" .
 	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
 	      "</a>\n";
 	print $cgi->a({-href => esc_param($home_link)}, $home_link_str) . " / ";

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-19 21:27 [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage Petr Baudis
@ 2006-09-23 12:57 ` Petr Baudis
  2006-09-23 19:36   ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Petr Baudis @ 2006-09-23 12:57 UTC (permalink / raw)
  To: junkio; +Cc: git

Dear diary, on Tue, Sep 19, 2006 at 11:27:25PM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> It provides more useful information for causual Git users than the Git docs
> (especially about where to get Git and such).
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>

Ping?  This is the only gitweb patch still in my stg stack. I guess
noone really cares strongly either way since there were no comments.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-23 12:57 ` Petr Baudis
@ 2006-09-23 19:36   ` Junio C Hamano
  2006-09-23 19:46     ` Petr Baudis
  2006-09-23 19:54     ` Jakub Narebski
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2006-09-23 19:36 UTC (permalink / raw)
  To: Petr Baudis; +Cc: junkio, git

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Tue, Sep 19, 2006 at 11:27:25PM CEST, I got a letter
> where Petr Baudis <pasky@suse.cz> said that...
>> It provides more useful information for causual Git users than the Git docs
>> (especially about where to get Git and such).
>> 
>> Signed-off-by: Petr Baudis <pasky@suse.cz>
>
> Ping?  This is the only gitweb patch still in my stg stack. I guess
> noone really cares strongly either way since there were no comments.

I did not care either way, but I did not like either of these
hardcoded strings in the code, and felt that if we are touching
that part of the code we also should be making real improvement
at the same time ;-).  

Doing something like this would let us update it easier, and 
let people override with GITWEB_CONFIG if they want to.

---

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3d06181..ce90178 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -51,6 +51,9 @@ our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
 
+our $githelp_url = "http://git.or.cz/";
+our $githelp_label = "git homepage";
+
 # source of projects list
 our $projects_list = "++GITWEB_LIST++";
 
@@ -1335,7 +1338,9 @@ EOF
 	print "</head>\n" .
 	      "<body>\n" .
 	      "<div class=\"page_header\">\n" .
-	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
+	      "<a href=\"" . esc_html($githelp_url) .
+	      "\" title=\"" . esc_html($githelp_label) .
+	      "\">" .
 	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
 	      "</a>\n";
 	print $cgi->a({-href => esc_param($home_link)}, $home_link_str) . " / ";

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-23 19:36   ` Junio C Hamano
@ 2006-09-23 19:46     ` Petr Baudis
  2006-10-05 20:47       ` Petr Baudis
  2006-09-23 19:54     ` Jakub Narebski
  1 sibling, 1 reply; 25+ messages in thread
From: Petr Baudis @ 2006-09-23 19:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Dear diary, on Sat, Sep 23, 2006 at 09:36:01PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > Dear diary, on Tue, Sep 19, 2006 at 11:27:25PM CEST, I got a letter
> > where Petr Baudis <pasky@suse.cz> said that...
> >> It provides more useful information for causual Git users than the Git docs
> >> (especially about where to get Git and such).
> >> 
> >> Signed-off-by: Petr Baudis <pasky@suse.cz>
> >
> > Ping?  This is the only gitweb patch still in my stg stack. I guess
> > noone really cares strongly either way since there were no comments.
> 
> I did not care either way, but I did not like either of these
> hardcoded strings in the code, and felt that if we are touching
> that part of the code we also should be making real improvement
> at the same time ;-).  
> 
> Doing something like this would let us update it easier, and 
> let people override with GITWEB_CONFIG if they want to.

Reasonable.

Acked-by: Petr Baudis <pasky@suse.cz>

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-23 19:36   ` Junio C Hamano
  2006-09-23 19:46     ` Petr Baudis
@ 2006-09-23 19:54     ` Jakub Narebski
  2006-09-23 20:46       ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Jakub Narebski @ 2006-09-23 19:54 UTC (permalink / raw)
  To: git

Junio C Hamano wrote:

> -             "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
> +             "<a href=\"" . esc_html($githelp_url) .
> +             "\" title=\"" . esc_html($githelp_label) .
> +             "\">" .

Why not use $cgi->a_begin({-href=>esc_param($githelp_url), -title=>$githelp_label});
or just plain $cgi->a and $cgi->img?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-23 19:54     ` Jakub Narebski
@ 2006-09-23 20:46       ` Junio C Hamano
  2006-10-06 10:31         ` [PATCH] gitweb: Cleanup Git logo and Git logo target generation Jakub Narebski
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-09-23 20:46 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>
>> -             "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
>> +             "<a href=\"" . esc_html($githelp_url) .
>> +             "\" title=\"" . esc_html($githelp_label) .
>> +             "\">" .
>
> Why not use $cgi->a_begin({-href=>esc_param($githelp_url), -title=>$githelp_label});
> or just plain $cgi->a and $cgi->img?

Be my guest and send in a proper patch please.

I was merely demonstrating my preference on how definition of
default values and actual use of them are separated.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-09-23 19:46     ` Petr Baudis
@ 2006-10-05 20:47       ` Petr Baudis
  2006-10-05 21:23         ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Petr Baudis @ 2006-10-05 20:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Dear diary, on Sat, Sep 23, 2006 at 09:46:43PM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> Dear diary, on Sat, Sep 23, 2006 at 09:36:01PM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
> > Petr Baudis <pasky@suse.cz> writes:
> > 
> > > Dear diary, on Tue, Sep 19, 2006 at 11:27:25PM CEST, I got a letter
> > > where Petr Baudis <pasky@suse.cz> said that...
> > >> It provides more useful information for causual Git users than the Git docs
> > >> (especially about where to get Git and such).
> > >> 
> > >> Signed-off-by: Petr Baudis <pasky@suse.cz>
> > >
> > > Ping?  This is the only gitweb patch still in my stg stack. I guess
> > > noone really cares strongly either way since there were no comments.
> > 
> > I did not care either way, but I did not like either of these
> > hardcoded strings in the code, and felt that if we are touching
> > that part of the code we also should be making real improvement
> > at the same time ;-).  
> > 
> > Doing something like this would let us update it easier, and 
> > let people override with GITWEB_CONFIG if they want to.
> 
> Reasonable.
> 
> Acked-by: Petr Baudis <pasky@suse.cz>

So, what happenned to this patch?

Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-10-05 20:47       ` Petr Baudis
@ 2006-10-05 21:23         ` Junio C Hamano
  2006-10-05 21:34           ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-10-05 21:23 UTC (permalink / raw)
  To: git

Petr Baudis <pasky@suse.cz> writes:

> Dear diary, on Sat, Sep 23, 2006 at 09:46:43PM CEST, I got a letter
> where Petr Baudis <pasky@suse.cz> said that...
>> Dear diary, on Sat, Sep 23, 2006 at 09:36:01PM CEST, I got a letter
>> where Junio C Hamano <junkio@cox.net> said that...
>> > Petr Baudis <pasky@suse.cz> writes:
>> > 
>> > > Dear diary, on Tue, Sep 19, 2006 at 11:27:25PM CEST, I got a letter
>> > > where Petr Baudis <pasky@suse.cz> said that...
>> > >> It provides more useful information for causual Git users than the Git docs
>> > >> (especially about where to get Git and such).
>> > >> 
>> > >> Signed-off-by: Petr Baudis <pasky@suse.cz>
>> > >
>> > > Ping?  This is the only gitweb patch still in my stg stack. I guess
>> > > noone really cares strongly either way since there were no comments.
>> > 
>> > I did not care either way, but I did not like either of these
>> > hardcoded strings in the code, and felt that if we are touching
>> > that part of the code we also should be making real improvement
>> > at the same time ;-).  
>> > 
>> > Doing something like this would let us update it easier, and 
>> > let people override with GITWEB_CONFIG if they want to.
>> 
>> Reasonable.
>> 
>> Acked-by: Petr Baudis <pasky@suse.cz>
>
> So, what happenned to this patch?

Did anybody send in an applicable patch?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-10-05 21:23         ` Junio C Hamano
@ 2006-10-05 21:34           ` Junio C Hamano
  2006-10-05 23:57             ` Petr Baudis
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-10-05 21:34 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git

Junio C Hamano <junkio@cox.net> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>>> > > Ping?  This is the only gitweb patch still in my stg stack. I guess
>>> > > noone really cares strongly either way since there were no comments.
>>> > 
>>> > I did not care either way, but I did not like either of these
>>> > hardcoded strings in the code, and felt that if we are touching
>>> > that part of the code we also should be making real improvement
>>> > at the same time ;-).  
>>> > 
>>> > Doing something like this would let us update it easier, and 
>>> > let people override with GITWEB_CONFIG if they want to.
>>> 
>>> Reasonable.
>>> 
>>> Acked-by: Petr Baudis <pasky@suse.cz>
>>
>> So, what happenned to this patch?
>
> Did anybody send in an applicable patch?

Ah, I found it; you do not have to re-send it.  But do you still
want it?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage
  2006-10-05 21:34           ` Junio C Hamano
@ 2006-10-05 23:57             ` Petr Baudis
  0 siblings, 0 replies; 25+ messages in thread
From: Petr Baudis @ 2006-10-05 23:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Dear diary, on Thu, Oct 05, 2006 at 11:34:42PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Junio C Hamano <junkio@cox.net> writes:
> 
> > Petr Baudis <pasky@suse.cz> writes:
> >
> >>> > > Ping?  This is the only gitweb patch still in my stg stack. I guess
> >>> > > noone really cares strongly either way since there were no comments.
> >>> > 
> >>> > I did not care either way, but I did not like either of these
> >>> > hardcoded strings in the code, and felt that if we are touching
> >>> > that part of the code we also should be making real improvement
> >>> > at the same time ;-).  
> >>> > 
> >>> > Doing something like this would let us update it easier, and 
> >>> > let people override with GITWEB_CONFIG if they want to.
> >>> 
> >>> Reasonable.
> >>> 
> >>> Acked-by: Petr Baudis <pasky@suse.cz>
> >>
> >> So, what happenned to this patch?
> >
> > Did anybody send in an applicable patch?
> 
> Ah, I found it; you do not have to re-send it.  But do you still
> want it?

Yes. :-) (I wouldn't prod about it otherwise.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-09-23 20:46       ` Junio C Hamano
@ 2006-10-06 10:31         ` Jakub Narebski
  2006-10-07  8:36           ` Junio C Hamano
  2006-10-08 13:31           ` Jakub Narebski
  0 siblings, 2 replies; 25+ messages in thread
From: Jakub Narebski @ 2006-10-06 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

Move styling of git logo <img> element from "style" attribute to CSS
via setting class to "logo".  Perhaps we should set it by id rather
than by class.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:

>>> +             "<a href=\"" . esc_html($githelp_url) .
>>> +             "\" title=\"" . esc_html($githelp_label) .
>>> +             "\">" .
>>
>> Why not use $cgi->a_begin({-href=>esc_param($githelp_url),
>> -title=>$githelp_label}); or just plain $cgi->a and $cgi->img?
>
> Be my guest and send in a proper patch please.
>
> I was merely demonstrating my preference on how definition of
> default values and actual use of them are separated.

And here it is. By the way, esc_html was incorrect, I think.

 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |   17 +++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 668e69a..3f62b6d 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -16,6 +16,11 @@ a:hover, a:visited, a:active {
 	color: #880000;
 }
 
+img.logo {
+	float: right;
+	border-width: 0px;
+}
+
 div.page_header {
 	height: 25px;
 	padding: 8px;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3320069..a966f9f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -46,13 +46,16 @@ our $home_text = "++GITWEB_HOMETEXT++";
 
 # URI of default stylesheet
 our $stylesheet = "++GITWEB_CSS++";
-# URI of GIT logo
+# URI of GIT logo (72x27 size)
 our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
 
-our $githelp_url = "http://git.or.cz/";
-our $githelp_label = "git homepage";
+# URI and label (title) of GIT logo link
+#our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
+#our $logo_label = "git documentation";
+our $logo_url = "http://git.or.cz/";
+our $logo_label = "git homepage";
 
 # source of projects list
 our $projects_list = "++GITWEB_LIST++";
@@ -1376,11 +1379,9 @@ EOF
 	print "</head>\n" .
 	      "<body>\n" .
 	      "<div class=\"page_header\">\n" .
-	      "<a href=\"" . esc_html($githelp_url) .
-	      "\" title=\"" . esc_html($githelp_label) .
-	      "\">" .
-	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
-	      "</a>\n";
+	      $cgi->a({-href => esc_url($logo_url),
+	               -title => $logo_label},
+	              qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
 	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
-- 
1.4.2.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-06 10:31         ` [PATCH] gitweb: Cleanup Git logo and Git logo target generation Jakub Narebski
@ 2006-10-07  8:36           ` Junio C Hamano
  2006-10-07  9:58             ` Jakub Narebski
  2006-10-08 13:31           ` Jakub Narebski
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-10-07  8:36 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> 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',
);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-07  8:36           ` Junio C Hamano
@ 2006-10-07  9:58             ` Jakub Narebski
  2006-10-07 14:14               ` Alan Chandler
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Narebski @ 2006-10-07  9:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano wrote:

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

You forgot to add style, i.e. either id => 'logo' or class => 'logo'. 

I'm not sure if it is worth complication. We have similar situation 
(although without image) with $home_link and $home_link_str (as with 
$logo, $logo_url, $logo_label).

I wonder how many people use non-standard logo image, or non standard 
favicon...
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-07  9:58             ` Jakub Narebski
@ 2006-10-07 14:14               ` Alan Chandler
  0 siblings, 0 replies; 25+ messages in thread
From: Alan Chandler @ 2006-10-07 14:14 UTC (permalink / raw)
  To: git

On Saturday 07 October 2006 10:58, Jakub Narebski wrote:

> I wonder how many people use non-standard logo image, or non standard
> favicon...

I'm using a non standard favicon to make it consistent with the rest of my 
site.

That said, I like the revised form.

-- 
Alan Chandler
http://www.chandlerfamily.org.uk

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-06 10:31         ` [PATCH] gitweb: Cleanup Git logo and Git logo target generation Jakub Narebski
  2006-10-07  8:36           ` Junio C Hamano
@ 2006-10-08 13:31           ` Jakub Narebski
  2006-10-08 19:47             ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Jakub Narebski @ 2006-10-08 13:31 UTC (permalink / raw)
  To: git

Was this patch dropped? Postponed to after release?
Waiting for better patch (but this correct esc_html-ing URL...)?

-- 
Jakub Narebski

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 13:31           ` Jakub Narebski
@ 2006-10-08 19:47             ` Junio C Hamano
  2006-10-08 20:10               ` Petr Baudis
  2006-10-08 20:17               ` Jakub Narebski
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2006-10-08 19:47 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> writes:

> Was this patch dropped? Postponed to after release?
> Waiting for better patch (but this correct esc_html-ing URL...)?

Pretty much the last one -- waiting for a patch directly
applicable from my e-mail client.  As far as I can recall there
has been none in the thread.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 19:47             ` Junio C Hamano
@ 2006-10-08 20:10               ` Petr Baudis
  2006-10-08 20:33                 ` Junio C Hamano
  2006-10-08 20:17               ` Jakub Narebski
  1 sibling, 1 reply; 25+ messages in thread
From: Petr Baudis @ 2006-10-08 20:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git

Dear diary, on Sun, Oct 08, 2006 at 09:47:22PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Was this patch dropped? Postponed to after release?
> > Waiting for better patch (but this correct esc_html-ing URL...)?
> 
> Pretty much the last one -- waiting for a patch directly
> applicable from my e-mail client.  As far as I can recall there
> has been none in the thread.

Is there a problem with taking <200610061231.06017.jnareb@gmail.com>?

I think it's currently not worth the complexity and breakage of
backwards compatibility to do the more elaborate form you proposed.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 19:47             ` Junio C Hamano
  2006-10-08 20:10               ` Petr Baudis
@ 2006-10-08 20:17               ` Jakub Narebski
  1 sibling, 0 replies; 25+ messages in thread
From: Jakub Narebski @ 2006-10-08 20:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

Move styling of git logo <img> element from "style" attribute to CSS
via setting class to "logo".  Perhaps we should set it by id rather
than by class.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I hope that this version is applicable...

 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |   17 +++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 668e69a..3f62b6d 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -16,6 +16,11 @@ a:hover, a:visited, a:active {
 	color: #880000;
 }
 
+img.logo {
+	float: right;
+	border-width: 0px;
+}
+
 div.page_header {
 	height: 25px;
 	padding: 8px;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3320069..a966f9f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -46,13 +46,16 @@ our $home_text = "++GITWEB_HOMETEXT++";
 
 # URI of default stylesheet
 our $stylesheet = "++GITWEB_CSS++";
-# URI of GIT logo
+# URI of GIT logo (72x27 size)
 our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
 
-our $githelp_url = "http://git.or.cz/";
-our $githelp_label = "git homepage";
+# URI and label (title) of GIT logo link
+#our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
+#our $logo_label = "git documentation";
+our $logo_url = "http://git.or.cz/";
+our $logo_label = "git homepage";
 
 # source of projects list
 our $projects_list = "++GITWEB_LIST++";
@@ -1376,11 +1379,9 @@ EOF
 	print "</head>\n" .
 	      "<body>\n" .
 	      "<div class=\"page_header\">\n" .
-	      "<a href=\"" . esc_html($githelp_url) .
-	      "\" title=\"" . esc_html($githelp_label) .
-	      "\">" .
-	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
-	      "</a>\n";
+	      $cgi->a({-href => esc_url($logo_url),
+	               -title => $logo_label},
+	              qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
 	print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
 	if (defined $project) {
 		print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
-- 
1.4.2.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 20:10               ` Petr Baudis
@ 2006-10-08 20:33                 ` Junio C Hamano
  2006-10-08 20:54                   ` Jakub Narebski
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-10-08 20:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Jakub Narebski, git

Petr Baudis <pasky@suse.cz> writes:

> Is there a problem with taking <200610061231.06017.jnareb@gmail.com>?
>
> I think it's currently not worth the complexity and breakage of
> backwards compatibility to do the more elaborate form you proposed.

I agree with that, except that the 72x27 dimension bit troubles
me.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 20:33                 ` Junio C Hamano
@ 2006-10-08 20:54                   ` Jakub Narebski
  2006-10-09  0:54                     ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Narebski @ 2006-10-08 20:54 UTC (permalink / raw)
  To: git

Junio C Hamano wrote:

> Petr Baudis <pasky@suse.cz> writes:
> 
>> Is there a problem with taking <200610061231.06017.jnareb@gmail.com>?
>>
>> I think it's currently not worth the complexity and breakage of
>> backwards compatibility to do the more elaborate form you proposed.
> 
> I agree with that, except that the 72x27 dimension bit troubles
> me.

First, that's the problem for the future. The 72x27 was there, I have not
introduced this. 

Second, 72x27 is size override, so although logo would better be 72x27,
but if it is not it will be scaled to given size.
  http://www.w3.org/TR/html401/struct/objects.html#visual
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-08 20:54                   ` Jakub Narebski
@ 2006-10-09  0:54                     ` Junio C Hamano
  2006-10-09  7:14                       ` Jakub Narebski
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2006-10-09  0:54 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>
>> Petr Baudis <pasky@suse.cz> writes:
>> 
>>> Is there a problem with taking <200610061231.06017.jnareb@gmail.com>?
>>>
>>> I think it's currently not worth the complexity and breakage of
>>> backwards compatibility to do the more elaborate form you proposed.
>> 
>> I agree with that, except that the 72x27 dimension bit troubles
>> me.
>
> First, that's the problem for the future. The 72x27 was there, I have not
> introduced this. 

That's exactly my point.  This is not a "placing blame" game.

It just feels wrong to update only two things when we already
know there are others that need fixing in a very similar way.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-09  0:54                     ` Junio C Hamano
@ 2006-10-09  7:14                       ` Jakub Narebski
  2006-10-09  9:00                         ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Jakub Narebski @ 2006-10-09  7:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Junio C Hamano wrote:
>>
>>> Petr Baudis <pasky@suse.cz> writes:
>>> 
>>>> Is there a problem with taking <200610061231.06017.jnareb@gmail.com>?
>>>>
>>>> I think it's currently not worth the complexity and breakage of
>>>> backwards compatibility to do the more elaborate form you proposed.
>>> 
>>> I agree with that, except that the 72x27 dimension bit troubles
>>> me.
>>
>> First, that's the problem for the future. The 72x27 was there, I have not
>> introduced this. 
> 
> That's exactly my point.  This is not a "placing blame" game.
> 
> It just feels wrong to update only two things when we already
> know there are others that need fixing in a very similar way.
 
Well, I'd rather have it corrected, than wait for generalized version
which would allow to set dimensions and stuff... which is most probably
not needed.

Partial improvement is better than no improvement (especially as it
corrects 'href="'.esc_html($githelp_url).'"' bug).
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-09  7:14                       ` Jakub Narebski
@ 2006-10-09  9:00                         ` Junio C Hamano
  2006-10-09  9:26                           ` Jakub Narebski
  2006-10-09 20:36                           ` Petr Baudis
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2006-10-09  9:00 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> writes:

> Partial improvement is better than no improvement.

Not necessarily.  As the maintainer, I found that when we say
"we will fix it later", later tend to never come, and one
effective way to fight that tendency is to prod the contributors
a bit harder, which worked reasonably well so far.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-09  9:00                         ` Junio C Hamano
@ 2006-10-09  9:26                           ` Jakub Narebski
  2006-10-09 20:36                           ` Petr Baudis
  1 sibling, 0 replies; 25+ messages in thread
From: Jakub Narebski @ 2006-10-09  9:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Partial improvement is better than no improvement.
> 
> Not necessarily.  As the maintainer, I found that when we say
> "we will fix it later", later tend to never come, and one
> effective way to fight that tendency is to prod the contributors
> a bit harder, which worked reasonably well so far.

Well. Perhaps.

But first, it is also bugfix for esc_html on URL, and second I though 
you rather have one feature per patch; this one has two - cleanup of 
logo generation, and moving style to CSS. "Better" patch would have 
three...

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] gitweb: Cleanup Git logo and Git logo target generation
  2006-10-09  9:00                         ` Junio C Hamano
  2006-10-09  9:26                           ` Jakub Narebski
@ 2006-10-09 20:36                           ` Petr Baudis
  1 sibling, 0 replies; 25+ messages in thread
From: Petr Baudis @ 2006-10-09 20:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git

Dear diary, on Mon, Oct 09, 2006 at 11:00:50AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Partial improvement is better than no improvement.
> 
> Not necessarily.  As the maintainer, I found that when we say
> "we will fix it later", later tend to never come, and one
> effective way to fight that tendency is to prod the contributors
> a bit harder, which worked reasonably well so far.

If later tends to never come, it means apparently noone really needs it.
:-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2006-10-09 20:36 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-19 21:27 [RFC][PATCH] gitweb: Make the Git logo link target to point to the homepage Petr Baudis
2006-09-23 12:57 ` Petr Baudis
2006-09-23 19:36   ` Junio C Hamano
2006-09-23 19:46     ` Petr Baudis
2006-10-05 20:47       ` Petr Baudis
2006-10-05 21:23         ` Junio C Hamano
2006-10-05 21:34           ` Junio C Hamano
2006-10-05 23:57             ` Petr Baudis
2006-09-23 19:54     ` Jakub Narebski
2006-09-23 20:46       ` Junio C Hamano
2006-10-06 10:31         ` [PATCH] gitweb: Cleanup Git logo and Git logo target generation Jakub Narebski
2006-10-07  8:36           ` Junio C Hamano
2006-10-07  9:58             ` Jakub Narebski
2006-10-07 14:14               ` Alan Chandler
2006-10-08 13:31           ` Jakub Narebski
2006-10-08 19:47             ` Junio C Hamano
2006-10-08 20:10               ` Petr Baudis
2006-10-08 20:33                 ` Junio C Hamano
2006-10-08 20:54                   ` Jakub Narebski
2006-10-09  0:54                     ` Junio C Hamano
2006-10-09  7:14                       ` Jakub Narebski
2006-10-09  9:00                         ` Junio C Hamano
2006-10-09  9:26                           ` Jakub Narebski
2006-10-09 20:36                           ` Petr Baudis
2006-10-08 20:17               ` Jakub Narebski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).